Skip to content

Nexus: Remove unused/useless variables from nexus_core - #6198

Closed
brockdyer03 wants to merge 5 commits into
QMCPACK:developfrom
brockdyer03:remove-unused-settings
Closed

Nexus: Remove unused/useless variables from nexus_core#6198
brockdyer03 wants to merge 5 commits into
QMCPACK:developfrom
brockdyer03:remove-unused-settings

Conversation

@brockdyer03

Copy link
Copy Markdown
Contributor

Proposed changes

I noticed that there were a significant number of variables in nexus_core that were either unused, constant and unchangeable, or redundant. The complete list of removed variables is given below:

  • modes
    • Defines allowed values for mode
  • mode
    • Always overridden to be the same value by Settings.process_core_settings()
  • stages_set
    • Created from stages, essentially always the same
  • stages
    • Created by mode
  • primary_modes
    • Only used to create stages
  • dependent_modes
    • Static, and only checked if it is a subset of stages_set, which is essentially always the case.
  • verbose
    • Always True.
  • debug
    • Only used to set verbose to True, but verbose is always True no matter what.
  • trace
    • Unused
  • status_modes
    • Defines allowed values for status
  • status
    • The only place this is checked is in ProjectManager.write_simulation_status(), but standard and none do the same thing, and since every status other than standard and none are basically unreachable via the user, this is useless.
  • emulate
    • Unused.

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

  • Refactoring (no functional changes, no api changes)
  • Testing changes (e.g. new unit/integration/performance tests)

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

Desktop, Fedora Linux 44 (KDE Plasma Desktop Edition)
AMD Ryzen 9 7900X (12 cores, 24 logical processors)

Python        3.14.5
uv            0.12.3
cif2cell      2.1.0
coverage      7.15.4
h5py          3.16.0
matplotlib    3.11.1
numpy         2.5.2
pycifrw       4.4.6
pydot         4.0.1
pytest        9.1.1
pytest-cov    7.1.0
pytest-order  1.5.0
scipy         1.18.0
seekpath      2.2.1
spglib        2.7.0
sphinx        9.1.0

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)

@brockdyer03
brockdyer03 requested a review from jtkrogel September 2, 2026 02:56
@github-actions github-actions Bot added nexus python Pull requests that update python code labels Sep 2, 2026
@jtkrogel

jtkrogel commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

--status active now results in the same output as --status_only. Fix.

@jtkrogel

jtkrogel commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

mode and stages are functional controls. Many of these call variants to drive specific functionality no longer work. Fix.

Static analysis (and partial runtime analysis) gives:

The affected settings(...) call forms are:

settings(mode='setup')
    Previously progressed only through input setup; it now raises ValueError.

settings(mode='send_files')
    Previously ran only the file-transfer stage; it now raises ValueError.

settings(mode='submit')
    Previously ran/monitored only job submission; it now raises ValueError.

settings(mode='get_output')
    Previously attempted only output retrieval; it now raises ValueError.

settings(mode='analyze')
    Previously attempted only analysis; it now raises ValueError.

settings(mode='none')
    Previously suppressed the main progression stages; it now raises ValueError.

settings(mode='all') or settings(mode='stages')
    These previously requested normal full progression and are behaviorally redundant with the default, but now raise ValueError, breaking compatible scripts.

settings(stages='setup')
    A single stage could previously be supplied as a string; it now raises ValueError.

settings(stages=['setup', 'send_files'])
    A selected sequence of stages could previously be supplied as a list; it now raises ValueError.

settings(stages=['all']) or settings(stages=[])
    These previously expanded to the default full stage list, but now raise ValueError despite being behaviorally equivalent to the default.

settings(mode='stages', stages=[...])
    This was the explicit form for selecting multiple stages and now raises ValueError.

settings(status=True) or settings(status='standard')
    These previously printed the complete workflow status before continuing execution; they now raise ValueError.

settings(status='active')
    This previously printed only active simulations before continuing; it now raises ValueError.

settings(status='ready')
    This previously printed only ready simulations before continuing; it now raises ValueError.

settings(status='failed')
    This previously printed only failed simulations before continuing; it now raises ValueError.

settings(status=False) or settings(status=None)
    These previously explicitly disabled the pre-run status display; they now raise ValueError, although omitting status has the same behavior.

settings(status_only=True, status='active'|'ready'|'failed')
    This previously printed the selected status subset and exited; it now raises ValueError, while unfiltered settings(status_only=True) remains supported.

settings(verbose=False)
    This previously suppressed NexusCore.log() output; it now raises ValueError, and logging is always enabled.

settings(verbose=True)
    This was equivalent to the default but accepted; it now raises ValueError.

settings(debug=True) or settings(debug=False)
    These were accepted but had no distinct effective behavior because normal verbosity was already enabled; they now raise ValueError.

settings(trace=True) or settings(trace=False)
    These were accepted no-ops; they now raise ValueError.

@jtkrogel

jtkrogel commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

The changes to verbose/debug result in errors and/or cause the logging to always be on. Fix.

Details:

Settings.allowed_vars validation before any settings are applied.

settings(verbose=False)
    Previously suppressed output from NexusCore.log(); it now raises ValueError: unrecognized variables provided ... ['verbose'], making quiet Nexus execution unavailable through this setting.

settings(verbose=True)
    Previously explicitly enabled normal logging; it now raises the same ValueError, breaking otherwise compatible scripts even though logging remains enabled by default.

settings(verbose=False, debug=False)
    Previously provided quiet execution explicitly; it now raises a ValueError listing both debug and verbose as unrecognized.

settings(verbose=False, debug=True)
    Previously caused debug=True to override verbose=False and enable logging; it now raises a ValueError before either setting is processed.

settings(debug=True)
    Previously was accepted and forced verbose=True, although verbosity was already enabled by default; it now raises ValueError: unrecognized variables provided ... ['debug'].

settings(debug=False)
    Previously was an accepted no-op/default declaration; it now raises the same ValueError, creating an API-compatibility break without removing useful behavior.

settings(trace=True)
    Previously was accepted but apparently unused by Nexus core; it now raises ValueError: unrecognized variables provided ... ['trace'].

settings(trace=False)
    Previously was an accepted no-op/default declaration; it now raises the same ValueError.

@jtkrogel jtkrogel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

…exus_core`, and simplify code that referenced them"

This reverts commit 5e35c6e.
@jtkrogel

jtkrogel commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

I expect the best way to address these issues is to revert the changes and then just reintroduce removals of the ones that actually do nothing.

@brockdyer03

Copy link
Copy Markdown
Contributor Author

The changes to verbose/debug result in errors and/or cause the logging to always be on. Fix.

The ability to turn off logging was undocumented, and additionally is not what verbose actually means in 99% of programs. verbose means extra output, not "any output at all".

debug was redundant/useless, and should be removed.

mode and stages are functional controls. Many of these call variants to drive specific functionality no longer work. Fix.

These settings were also undocumented, untested, and produced unexpected behavior. While they may have served a purpose, the best plan is to remove them for now and consider possibly replacing them in the future. As of now they just introduce more legacy code baggage that isn't tested or documented.

Here are responses to why these are not useful/expected:

settings(mode='setup')
settings(mode='send_files')
settings(mode='submit')
settings(mode='get_output')
settings(mode='analyze')
settings(mode='none')
settings(mode='all') or settings(mode='stages')

These did nothing to actually change mode, and they in fact would always override stages if that were set. Redundant.

settings(stages='setup')

This doesn't actually fully set up a calculation, you need send_files to get a real calculation, and if you added that then it was the same as using generate_only=True.

settings(stages=['setup', 'send_files'])

This is equivalent to generate_only=True.

settings(stages=['all']) or settings(stages=[])

Undocumented no-op, useless.

settings(mode='stages', stages=[...])

This was redundant and never documented.

@jtkrogel

jtkrogel commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

The best solution for features that are not understood, but are useful, is documentation.

We should include something like this in the manual rather than ripping out intentional, and intentionally preserved, functionality:

nexus_core_feature_docs.txt

Please limit this PR to true non-functional (does nothing) variables.

@brockdyer03

Copy link
Copy Markdown
Contributor Author

Closing because it is replaced by #6207.

@brockdyer03 brockdyer03 closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nexus python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants