Refine find_nexus_module search locations and error handling #5767

ye-luo · 2026-01-28T22:09:12Z

Proposed changes

Currently, nexus executables require a closely coupled nexus modules, namely

<nexus_root>/bin # contains nexus exectuables
<nexus_root>/nexus #contains nexus module files

find_nexus_module honors this layout and verifies nexus module import.
nxs-test has the gold implementation and other scripts follow.

What type(s) of changes does this code introduce?

Refactoring (no functional changes, no api changes)
Other (please describe): more error traps

Does this introduce a breaking change?

No

What systems has this change been tested on?

laptop

Checklist

- I have read the pull request guidance and develop docs
- This PR is up to date with the current state of 'develop'
- Code added or changed in the PR has been clang-formatted
- This PR adds tests to cover any new code, or to catch a bug that is being fixed
- Documentation has been added (if appropriate)

ye-luo · 2026-01-28T22:09:24Z

Test this please

brockdyer03

As best I can tell, while this code does unify the find_nexus_modules function, it doesn't do anything to stop enforcement of the location of the executables being with the rest of Nexus. Perhaps a separate bit of code that checks if you're in a QMCPACK install directory would fix it?

nexus/bin/nexus_modules.py

brockdyer03 · 2026-01-28T23:31:23Z

nexus/bin/nexus_modules.py

+    nexus_lib2 = os.path.realpath(os.path.join(nexus.__file__,'..','..'))
+
+    # ensure closely coupled nexus module is in use.
+    if nexus_lib != nexus_lib2:


This is the line that is probably going to continue failing if the executables get moved. The enforcement of location of the current file with the location of Nexus means that this will likely still result in a failure if the executables are moved.

Unfortunately that is a current requirement.

We should keep the strict association in this PR. We can remove it in a following PR once a decision is made.

ye-luo · 2026-01-29T00:51:12Z

The introduced nexus_modules.py generate __pycache__ and prevents executing nexus/install.

qmcpack/nexus/install

Line 116 in 01c9846

execute('cp {0}/bin/* {1}'.format(source_dir,install_dir))

err: b"cp: -r not specified; omitting directory '/home/yeluo/opt/qmcpack/nexus/bin/__pycache__'\n"

brockdyer03 · 2026-01-29T01:22:02Z

To be honest I forgot about __pycache__. That gets created whenever you import Python modules that are not your current module. Usually that only gets made in qmcpack/nexus/nexus, but since there was a .py file added to qmcpack/nexus/bin it'll make one if you run anything before doing the copy. I don't think that creating a separate .py file is a viable option.

I think since @jtkrogel said he was fine with most of the executables not actually enforcing association with the rest of Nexus (except for nxs-test) it would probably be fine to just delete find_nexus_modules from all of the files (again, except for nxs-test) and put the onus on the user to ensure their Python path is set properly.

…y nexus exectuables.

ye-luo · 2026-01-29T04:35:34Z

To be honest I forgot about __pycache__. That gets created whenever you import Python modules that are not your current module. Usually that only gets made in qmcpack/nexus/nexus, but since there was a .py file added to qmcpack/nexus/bin it'll make one if you run anything before doing the copy.

You analysis is correct

I don't think that creating a separate .py file is a viable option.

simply avoid files starting with underscore. taken care by #5768

I think since @jtkrogel said he was fine with most of the executables not actually enforcing association with the rest of Nexus (except for nxs-test) it would probably be fine to just delete find_nexus_modules from all of the files (again, except for nxs-test) and put the onus on the user to ensure their Python path is set properly.

I actually prefer all the nexus executable runnable without setting PYTHONPATH by the user or by a virtual environment. They must be exactly matched to the nexus module in the same git commit. The current strict coupling requirement made nexus executables invulnerable to hidden PYTHONPATH changes.

jtkrogel · 2026-01-29T10:41:15Z

Need to catch up with the discussion here. From what I understand of our prior discussion, no decisions had been made about enforcing strict association between the binaries and nexus modules (though I favor retaining it).

jtkrogel · 2026-01-29T10:50:02Z

On a closer look, I really don't see the virtue of including a non-binary (find_nexus_modules.py) in the binaries directory. I can understand the desire to remove all instances of code duplication, but I think the unwieldiness of this change is worse.

brockdyer03 · 2026-01-29T11:08:14Z

simply avoid files starting with underscore. taken care by #5768

I think this could potentially introduce unwanted side effects. For example, qmcpack/nexus/nexus has a file __init__.py which was renamed from nexus.py to be compliant with packaging guidelines. Without that file, the entire package will stop working, so if that copy script ever gets duplicated it will cause significant errors that will be hard to trace.

prckent

nexus/bin is for directly shell executable programs; putting a not directly callable function there is non-standard and will confuse users (etc.). Could it be made a verify_nexus_installation utility that can also be called directly? If this route works, we could expand it subsequently to include a reporting of the various nexus required and optional package imports.

…in nxs-test

ye-luo · 2026-01-29T18:55:14Z

@prckent verify_nexus_installation has been introduced. I couldn't find a clean way to avoid code duplication. Using sub-process to invoke verify_nexus_installation is still very messy. So just copied find_nexus_module into each script noting that the reference copy is from verify_nexus_installation

jtkrogel · 2026-02-02T17:27:24Z

It is the job of nxs-test to validate the installation, so having verify_nexus_installation is redundant and misleading (it verifies nothing but paths while the installation could still be broken). The "gold" implementation of the path checking should reside in nxs-test.

jtkrogel · 2026-01-29T10:43:18Z

nexus/bin/nexus_modules.py

+    nexus_lib2 = os.path.realpath(os.path.join(nexus.__file__,'..','..'))
+
+    # ensure closely coupled nexus module is in use.
+    if nexus_lib != nexus_lib2:


We should keep the strict association in this PR. We can remove it in a following PR once a decision is made.

jtkrogel · 2026-01-29T10:44:22Z

nexus/bin/nxs-test

    from time import time # used to get user wall clock rather than cpu time
    tstart_all = time()

-    testing_wrong_nexus_install = False


Why is this deleted?

Please reply. This should not be removed.

The replaced find_nexus_module did the work already.

Due to the current requirement of nxs-test that exception should not be raise by itself. The gold implementation can not be used and kept by nxs-test. I reverted most of the changes in nxs-test.

jtkrogel · 2026-02-02T17:29:03Z

nexus/bin/verify_nexus_installation

@@ -0,0 +1,38 @@
+#! /usr/bin/env python3


Should be removed. See comment in the conversation/chat.

ye-luo · 2026-02-02T20:49:22Z

It is the job of nxs-test to validate the installation, so having verify_nexus_installation is redundant and misleading (it verifies nothing but paths while the installation could still be broken). The "gold" implementation of the path checking should reside in nxs-test.

Currently nxs-test needs extra work to have a proper installation. Consider the added tool a stopgap utility.

…executables

ye-luo · 2026-02-03T22:33:48Z

@prckent I removed verify_nexus_Installation that @jtkrogel considered redundant and we had a hard time to agree on what exactly it should serve and where it should be hosted. The same check exists in every nexus binary should be sufficient to catch the issue and inform users.

brockdyer03 reviewed Jan 28, 2026

View reviewed changes

ye-luo marked this pull request as draft January 29, 2026 00:45

ye-luo force-pushed the consolidate-nexus-executables branch 2 times, most recently from 149eedb to 5d9f62e Compare January 29, 2026 01:18

Introduce nexus_modules to handle consistent nexus module searching b…

0441e82

…y nexus exectuables.

ye-luo force-pushed the consolidate-nexus-executables branch from 5d9f62e to 0441e82 Compare January 29, 2026 04:17

ye-luo marked this pull request as ready for review January 29, 2026 04:41

prckent requested changes Jan 29, 2026

View reviewed changes

Keep find_nexus_module within each Nexus script. Gold implementation …

bd9bd9f

…in nxs-test

ye-luo changed the title ~~Introduce nexus_modules to handle consistent nexus module searching by nexus exectuables~~ Refine find_nexus_module search locations and error handling Jan 29, 2026

ye-luo force-pushed the consolidate-nexus-executables branch from 4657b08 to 139fac4 Compare January 29, 2026 18:51

ye-luo force-pushed the consolidate-nexus-executables branch from 139fac4 to 5eec27e Compare January 29, 2026 18:55

Introduce verify_nexus_installation

646ad70

ye-luo force-pushed the consolidate-nexus-executables branch from 5eec27e to 646ad70 Compare January 29, 2026 19:01

ye-luo added this to the v4.2.0 Release milestone Jan 29, 2026

jtkrogel requested changes Feb 2, 2026

View reviewed changes

ye-luo added 3 commits February 3, 2026 15:44

Merge remote-tracking branch 'origin/develop' into consolidate-nexus-…

27ea395

…executables

Revert changes in nxs-test

4ada190

Remove verify_nexus_installation

c5f7527

Refine find_nexus_module search locations and error handling #5767

Are you sure you want to change the base?

Refine find_nexus_module search locations and error handling #5767

Conversation

ye-luo commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

What type(s) of changes does this code introduce?

Does this introduce a breaking change?

What systems has this change been tested on?

Checklist

Uh oh!

ye-luo commented Jan 28, 2026

Uh oh!

brockdyer03 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ye-luo commented Jan 29, 2026

Uh oh!

brockdyer03 commented Jan 29, 2026

Uh oh!

ye-luo commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtkrogel commented Jan 29, 2026

Uh oh!

jtkrogel commented Jan 29, 2026

Uh oh!

brockdyer03 commented Jan 29, 2026

Uh oh!

prckent left a comment

Choose a reason for hiding this comment

Uh oh!

ye-luo commented Jan 29, 2026

Uh oh!

jtkrogel commented Feb 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ye-luo commented Feb 2, 2026

Uh oh!

ye-luo commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ye-luo commented Jan 28, 2026 •

edited

Loading

ye-luo commented Jan 29, 2026 •

edited

Loading

ye-luo commented Feb 3, 2026 •

edited

Loading