Preserve args in Level string representation for frequency scaling - #972
Draft
calvinp0 wants to merge 1 commit into
Draft
Preserve args in Level string representation for frequency scaling#972calvinp0 wants to merge 1 commit into
calvinp0 wants to merge 1 commit into
Conversation
…factor
`Level.__init__` normalises `args` into both a `keyword` and a `block` slot, so
a keyword-only level (the common case) has an empty `block` slot and
`all(val for val in self.args.values())` is False. `__str__` therefore dropped
args entirely: two levels differing only in args compared equal (`__eq__` is
str-based) while their `as_dict()` differed. The predicate now uses `any()`.
Flipping that predicate alone silently changes a physical quantity.
`assign_frequency_scale_factor()` keys on `str(level)` and returns None on a
miss. All 67 keys in `data/freq_scale_factors.yml` have the form
`method/basis[, solvation_method: ..., solvent: ..., software: ...]`; none
carries args. Once args entered the string, every args-carrying level lost its
scale factor, and since `calc_freq_factor` defaults to True that miss spawns a
full campaign of QM jobs to recompute the factor by Truhlar's method.
The database is keyed on the physical level of theory - method, basis,
dispersion, solvation, software - whereas args are ESS input-file directives
(extra route keywords and input blocks) that the database has never
distinguished. The lookup key is therefore built from a reduced string rather
than from `str(level)`: `get_freq_scale_factor_key()` hands the same key to both
consumers, `assign_frequency_scale_factor()` and
`output._resolve_freq_scale_factor_source()`.
`args` are the only attribute dropped. Everything else the string carries stays
in the key, including attributes the database happens not to use: `dispersion`,
`auxiliary_basis` and `cabs` are part of the model chemistry, and `year` and
`software_version` select a specific parameterisation elsewhere in ARC, so for
all of them "no published factor for this level" is the true answer rather than
a reason to widen the match. Keying on something narrower, such as
`Level.simple()`, would match nothing at all, since every key carries
`software:` and `simple()` omits it.
A level with both arg slots populated missed the database even before this
change and now resolves, consistent with the keyword-only and block-only cases.
The printing loop only ever emitted the `keyword` slot, so a block-only level
would have printed `keyword args: {}` - meaningless output and a broken key.
Every non-empty slot is now printed under its own name.
`utils/scale.py` prints a level as a yaml key for pasting into the database;
that key now comes from the same helper. Its project folder name still derives
from `str(level)`, so levels differing only in args keep separate directories,
but `rename_level()` substituted only `/ * + ( ) space :` and now maps every
remaining character outside `settings['valid_chars']` - the set ARC already
requires of a project name - onto `_`. Args reach that name and can carry
`{ } ' % = \ " $` and backticks, and the name is joined to a path that is passed
to `shutil.rmtree()` and interpolated into double-quoted shell commands.
`__eq__` is str-based, so levels differing only in args no longer compare equal.
Audited: the scheduler's "sp reuses the opt output" shortcut and its
conf_sp/conf_opt comparison now treat args-differing levels as different, which
is the conservative direction - an extra job rather than reusing an output
computed under different directives. Pipe batching is unaffected: `TaskSpec.level`
is a `Level.as_dict()` dict, and the planner compares dicts, not strings.
`as_dict()` is untouched here and still drops keyword-only args, so on its own
this change leaves `__str__` and `as_dict()` asymmetric in the opposite
direction to the one described above. The `as_dict()` predicate is fixed
separately in the branch that preserves args through the as_dict round-trip; the
two are independent and merge cleanly.
Looked for an existing normaliser before adding one: nothing strips args.
`Level.simple()` drops software and solvation, `statmech.arkane._level_to_str()`
builds Arkane `LevelOfTheory` strings, and `scale.rename_level()` only sanitises
a string for folder naming.
calvinp0
force-pushed
the
fix_level_str_args
branch
from
August 12, 2026 11:16
282d7f0 to
143a0c2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #972 +/- ##
==========================================
- Coverage 63.81% 63.81% -0.01%
==========================================
Files 119 119
Lines 39379 39386 +7
Branches 10224 10225 +1
==========================================
+ Hits 25131 25135 +4
- Misses 11265 11271 +6
+ Partials 2983 2980 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.