Skip to content

Commit dc000c5

Browse files
andyclaude
andcommitted
nwchemrx.desc: MASSES must precede the entries that read it (#126)
Follow-up to making the monitor's iteration deterministic. Auditing every parser script that reads a file a sibling writes turned up three such pairs: parseSym nwchem.symlab -> nwchem.molab nwchem.desc OK parseMD nwchem.mdheader -> nwchem.mdprop nwchemmd.desc OK rxMasses nwchemrx.masses -> nwchemrx.rxstep nwchemrx.desc WRONG nwchemrx.desc had the reader first: [TSSTEP] and [RXSTEP] at lines 17 and 24, [MASSES] at 31. Under the old hash-order iteration that was another coin flip; with the order now fixed it would have been permanently broken instead, which is worse. Moved [MASSES] above both. The failure is not silent-but-harmless: nwchemrx.rxstep divides by sqrt of each mass to un-mass-weight its coordinates, so with the file absent the array is empty and the script dies on a division by zero. The property is simply never stored. Added a check to tests/parsers that derives this from the same table, so a new .desc cannot reintroduce it. Verified it actually fails: against the previous ordering it reports both reader entries by line number, and passes once MASSES is moved. All four suites pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EFMUNbw2UpRPmgsqviAowb
1 parent 1294551 commit dc000c5

2 files changed

Lines changed: 63 additions & 6 deletions

File tree

‎scripts/parsers/nwchemrx.desc‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,27 @@ Frequency=last
1414
End= Frequencies
1515
[END]
1616

17+
# MUST COME BEFORE [TSSTEP] AND [RXSTEP]. nwchemrx.masses writes
18+
# $key/rxMasses, and nwchemrx.rxstep divides by sqrt of those masses to
19+
# un-mass-weight its coordinates -- with the file absent the array is
20+
# empty and the script dies on a division by zero, so the property is
21+
# simply never stored.
22+
#
23+
# eccejobmonitor flushes buffered blocks in the order parse types
24+
# appear in THIS FILE, and the client runs each script in arrival
25+
# order, so the ordering here is the dependency. It used to iterate a
26+
# Perl hash instead, which randomized the order per process and made
27+
# this pair work or fail by coin flip (see the NWChem SYMLAB/MOLAB
28+
# case, which had the identical shape). Reordered when that was fixed:
29+
# a deterministic wrong order would have been permanently broken rather
30+
# than intermittently.
31+
[MASSES]
32+
Script=nwchemrx.masses
33+
Begin = Mass of atom
34+
Frequency=first
35+
End = Forward reduced mass
36+
[END]
37+
1738
[TSSTEP]
1839
Script=nwchemrx.rxstep
1940
Begin= All quantities in atomic units, x is vector of mass-weighted coordinates
@@ -28,9 +49,3 @@ Frequency=all
2849
End = (\s*\S*\s*dx=)
2950
[END]
3051

31-
[MASSES]
32-
Script=nwchemrx.masses
33-
Begin = Mass of atom
34-
Frequency=first
35-
End = Forward reduced mass
36-
[END]

‎tests/parsers/run_tests.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,51 @@ def norm(text):
7979
# .desc structural checks -- run once per .desc file, independent of fixtures
8080
# ---------------------------------------------------------------------------
8181

82+
# Parser scripts that read a file a SIBLING script writes. The monitor
83+
# flushes buffered blocks in descriptor-file order and the client runs
84+
# each script as its block arrives, so the writer's entry must appear
85+
# before the reader's or the reader runs against a file that does not
86+
# exist yet.
87+
#
88+
# This was a live bug (#126): the monitor used to iterate a Perl hash
89+
# instead, randomizing the order per process, so NWChem orbital
90+
# symmetries were extracted or silently dropped roughly three runs in
91+
# eight. Making the order deterministic fixes that -- and makes a
92+
# wrongly-ordered .desc permanently broken instead of intermittently
93+
# working, which is why this check exists.
94+
AUX_FILE_DEPENDENCIES = {
95+
# aux file writer script reader script
96+
'parseSym': ('nwchem.symlab', 'nwchem.molab'),
97+
'parseMD': ('nwchem.mdheader', 'nwchem.mdprop'),
98+
'rxMasses': ('nwchemrx.masses', 'nwchemrx.rxstep'),
99+
}
100+
101+
102+
def check_aux_file_order(desc_name, desc, res):
103+
"""A script's auxiliary-file writer must precede its readers."""
104+
entries = list(desc.live_entries())
105+
for aux, (writer, reader) in AUX_FILE_DEPENDENCIES.items():
106+
wlines = [e.line for e in entries if e.script == writer]
107+
rlines = [e.line for e in entries if e.script == reader]
108+
if not wlines or not rlines:
109+
continue
110+
first_writer = min(wlines)
111+
for rline in rlines:
112+
res.check(
113+
first_writer < rline,
114+
desc_name,
115+
'%s writes %s and %s reads it, but the reader\'s entry is at '
116+
'line %d, before the writer at line %d. The monitor flushes '
117+
'in this file\'s order, so the reader will run against a '
118+
'file that does not exist yet -- silently (see #126).'
119+
% (writer, aux, reader, rline, first_writer))
120+
121+
82122
def check_desc_structure(desc_name, desc, res):
83123
where = desc_name
84124

125+
check_aux_file_order(desc_name, desc, res)
126+
85127
for entry, which, err in desc.bad_regex:
86128
res.check(False, where,
87129
'[%s] (line %d): %s= pattern does not compile: %s'

0 commit comments

Comments
 (0)