Stop giving wrong or incomplete results without saying so by Austin243 · Pull Request #86 · Austin243/ReactionFlow

Austin243 · 2026-11-06T19:41:06Z

Summary

Fixes the places where ReactionFlow could give a wrong or incomplete result without saying so. On a healthy run no frame, event, barrier or result file changes. What changes is what the commands report, what they refuse, and one new stop for a dead EON model.

Adds a net 621 lines in src (+769/−148). Review commit by commit: each commit is one fix, and ruff and the tests pass at each. Any commit can be dropped on its own terms, though later status commits build on earlier ones.

Before	Now	Commit	Lines
A deleted or empty segment: `completed`, exit 0, frames gone with no message	MD continues, since the checkpoint is unaffected. `run` warns and its last line has `missing_frames`	`067deb8`	+77
A cut current segment: raw `JSONDecodeError` on every resume	The same refusal, naming the file and the way out: restore it, or move it aside	`067deb8`
A cut earlier segment: the next refinement failed the run for good	Refinement skips it, and the run finishes and reports the frames	`067deb8`
`scan` with a deleted first segment: frames 0, exit 0, no message. A cut offset table: raw `ValueError`. Results with no segments: "not started"	Reads what is left and lists the gaps in the summary and on stderr; says "no saved frames"	`3d16a34`	+35
`status` with one unreadable file: events 0 and all the trajectory's reactions dropped, so a copy of a run's results alone looked empty	Each file costs only what depends on it. Counts and barriers stay, with a line saying the report is incomplete	`e90d4ee`, `d525749`	+67, +14
EON searches failing: `run` silent, `status` only `{"failed": N}`	`status` counts failures by reason, `run` prints each new reason up to ten, and the error that ended a run stays in `last-error.json`	`ceb8733`	+71
EON, a model that breaks after the start relaxes: runs until the job's time limit, writing failed records	Exit 1, nothing recorded for that search, and a rerun repeats it with its saved seed	`0c551bc`	+57
EON, Ctrl-C: did not stop the run, and wrote false failed records that changed later ones	Stops within about a second, and a rerun matches an uninterrupted run	`0c551bc`
`check` on images with different atoms: `saddle_validated`. A permuted saddle image: a real saddle came back `no_saddle`. A NaN first-image energy: validated, barrier NaN	Mismatched atoms refused, images with IDs reordered, NaN gives `saddle_unchecked`	`0ded79f`	+28
`check` validated a highest image carrying 0.545 eV/Å, and a barrier 6.7 times too low from unrelaxed ends, with no notice	The verdict is unchanged. The largest force on the first, highest and last images is always reported, and the message states them when past tolerance	`0e0364f`	+44
Region radius 0: a whole run finds nothing, exit 0	Refused at `validate`, `run` and `scan`. `status --radius 0` is a different radius and still works	`5abadec`	+6
`status` barrier row: `0.072 / 0.350 / 0.627`, where the median is the barrier of nothing	Split by direction: `0.627 / 0.627 / 0.627`, reverse `0.072 / 0.072 / 0.072` (truth 0.628 and 0.073)	`d747789`	+47
`status` was silent on why events got no band, on events still queued, and on a resume under another environment	All three shown, including a changed calculator source	`6b350cf`, `db6fc54`, `6fd9152`	+17, +7, +20
Silent settings: a multi-frame structure starts from its last frame; momenta gave 34,845 K at a 10 K target; FixAtoms dropped in refinement; an NPT cell refinement cannot use; a run too short to confirm anything	`validate` and a trajectory's first `run` warn. Nothing stops	`d246284`, `96223b5`	+49, +9
`scan` of a LAMMPS dump with only types: carbon read as hydrogen, a false zero. A file with no cell: false bonds on wrap	Warns; the counts do not change	`ae1aeb5`, `964c4d1`	+48
`run()` on a completed run with a larger target did nothing	Refused	`9b0d780`	+6
A runtime restoring other atoms than its checkpoint holds was accepted, though the docs say refused	Refused	`edd6c73`	+4
NaN energy with finite forces passed the model check, though the docs say the job stops	Stops, with the queue intact	`1849e2d`	+5
One bad saved frame failed the whole trajectory, and a later failure could store the wrong reactant	That event ends `failed`, with the right reactant	`f500cd4`	+1
`eon.displace_centers` ignored with `displace: all`, the default	Refused	`133425a`	+5
A model file replaced mid-run on the generic adapter mixed two potentials	Refused, as on the Torch adapters	`76846a0`	+3
A `candidates.csv` row labelled by its class's first-seen direction	Labelled by its own	`867b1a3`	+1
Docs: a strict restart mode that does not exist; xTB restart called rounding-level; a changed device said to continue; "Nothing is overwritten"	Corrected, with xTB's measured numbers	`1f8bc72`	0

What to look at when merging

EON campaigns in progress refuse to resume, because eon.py and runner.py are hashed into a campaign's contract. Finish one first, or keep it on its old install.
MD runs resume normally. Six runs killed before these commits and resumed on them finished with no alarm and the same reactions.
status --json: barriers_eV now holds only the direction a row names when a class runs both ways, and reverse_barriers_eV the other. A script wanting the old list adds the two. A symmetric reaction has the same graph both ways, so its row does not change.
New refusals: radius 0, displace_centers with displace: all, a band with mismatched atoms, and extending a completed run through run().
The EON stop is the only new automatic stop. Its condition: the model raised or returned a non-finite value during a search, and fails again on that search's starting structure, which it evaluated before. There is no count or threshold. A model that passes that check keeps running, and the record says what it raised.

Left out on purpose

These change a default or a scientific acceptance rule, or need testing not available here, so they are not in this PR:

whether check should withhold a barrier past a force tolerance
whether refinement should keep FixAtoms
init's default observation interval of 100 steps, against 10 in both examples
EON's own max-energy and negative-barrier checks on guided searches
a non-finite guard inside refinement
a cap on failed EON searches when the model is fine
a run-time exactness test at each restore
a lock on an MD trajectory
the barrier's first-image reference

Testing

Comparison with the original: 629 cases compared on exit code, stdout, stderr and every file written. 284 are identical, and the other 345 were read by group. They are the reporting and refusals above, plus one damaged run that now finishes. No check verdict or barrier changed on any band. At the oldest supported versions (ASE 3.28, numpy 1.26), the same 316 of 589 cases differ.
Real EON: pyeonclient 0.4.2 on a Linux login node. A dead model went from 880 failed searches and 3.7 MB in 8 s, still running, to exit 1 in 2 s. After Ctrl-C, a rerun matched an uninterrupted run in 45 of 45 searches, against 42 or 43 before. The 11 tests in eon.yml pass.
Failure modes first: each fix's ways of failing were written down, then tested in isolation on both trees.
Independent attack by running code, about 2,000 comparisons: it found one regression in this branch and nine smaller problems, all fixed here.
Not run: GPU, real Slurm or srun, Lustre, power loss, and real EON with a real MLIP.

…nt cannot be read A deleted or empty segment went unnoticed: the run ended completed with exit 0 and the frames gone. A cut tail stopped every resume with a raw JSONDecodeError, and a cut earlier segment failed the run for good at its next refinement. Now run warns and its last line names the missing frames, a cut current segment is refused with the file and the way out, and refinement skips a segment it cannot read. MD continues, since lost frames cannot change the exact state in the checkpoint.

A deleted first segment gave frames 0 with exit 0 and no message, and a missing middle segment hid every later frame. A cut offset table raised a raw ValueError, a trajectory with results and no segments said not started, and segments without state.json raised FileNotFoundError. Now the scan reads what is left and lists what it could not read in the summary and on stderr.

…be read One unreadable bundle or result.json dropped all of a trajectory's reactions and showed events 0, so a copy of a run's results alone looked empty, and a missing database looked like no reactions at all. Each file now costs only what depends on it. Another occurrence's bundle stands for its class, a class with none keeps its counts and barriers in its own row, and the error column and a line under the table say what was left out. Lost saved frames show there too.

A run whose searches all failed printed nothing, and status showed only a failed count at confidence 0; the reason was only in attempts/*.json. A run that ended in an error left no trace in status, and status said not started when records existed without search-state.json. Now status counts failed searches by reason, run prints each new reason on stderr up to ten, and the error that ended a run is kept in last-error.json and shown by status until the next run starts. Whether a run should stop by itself is a separate decision; this only reports.

A last image with 3 atoms or a middle image with O2 validated with no complaint, and a saddle image in another atom order displaced the wrong atoms: a real saddle came back no_saddle. A NaN energy on the first image gave saddle_validated with a NaN barrier. Images now must hold the same atoms, elements and periodicity, images with IDs are put in the first image's order, a non-finite energy ends as saddle_unchecked, and a pressure on a band with no cell says so.

check said saddle_validated for a highest image with a force of 0.545 eV/Å and for unrelaxed ends that made a barrier 6.7 times too low. The first force sat deep in the JSON and the second nowhere. check now records the largest atomic force on the first, highest and last images, and a validated band's message says when they pass neb_fmax or relax_fmax. The verdict and barrier do not change: whether check should withhold them is a scientific acceptance threshold, left open.

Radius 0 passed validate and a whole run then found nothing, with exit 0: a region of single atoms has no bond in it to change. Real frames gave 0 candidates against 36. Making 0 work would only move the emptiness to refinement, whose endpoint check compares bonds a region of changed atoms leaves out. status --radius 0 and same_reaction(radius=0) are a different radius, for classes, and stay.

A class holds both directions of a reaction, and each barrier is measured from its own event's reactant, so min, median and max ran over both under one label: 0.072 / 0.350 / 0.627 at 20 GPa, where the truth is 0.628 one way and 0.073 the other, and the median is the barrier of nothing. Now the barrier is for the direction the row names, and a further column, reverse_barriers_eV in --json, for the other. A symmetric reaction has the same graph both ways and stays one list, so its row does not change. A barrier whose event cannot be read goes to unknown_direction_barriers_eV.

run() on a completed run with a larger total_steps returned the old summary with no message and ran no MD, so a Python caller could read it as the longer run. The command line never reached this, because the campaign contract refuses a changed total_steps first.

docs/exact-restart.md says such a provider is refused, but only the step counter was compared at a restore: a provider shifting atoms by 0.123 Å ran to completion and looked like an exact continuation. The refusal leaves the checkpoint in place, so the run resumes once the provider is fixed.

The check after a refinement with no barrier tested forces only, so a model returning NaN energy with finite forces completed with exit 0 and every event neb_failed, with no mention of NaN. The campaign docs say the job stops if the model fails or returns NaN.

…t reactant The search for the reactant frame ran outside the refinement's try, so a saved frame with a singular cell or no IDs marked the whole trajectory failed for good, not that one event. And after falling back from an earlier frame to the tracker's, a later failure stored the earlier frame as the reactant.

These all ran with exit 0 and no message: a multi-frame structure started from its last frame; momenta in the structure gave 34845 K at a 10 K target; FixAtoms held in MD and was dropped in refinement; a left-handed or partly periodic NPT cell ran all MD and then refined nothing; and a run too short to confirm any reaction reported none. validate and a trajectory's first start now say so. Nothing here stops a run.

A LAMMPS dump with id, type and coordinates reads carbon as hydrogen, and reported 0 bond changes with exit 0 where the file holds a C-C break: a false zero on the commonest dump layout. A file with no cell gave a false formed bond when an atom wrapped at a boundary. The counts do not change.

displace_centers applies only to a local push, and displace defaults to all, so a campaign setting only the centers pushed every atom without a word, and its exploration was not the restricted one it asked for.

The generic adapter hashed its model files once per process, so a file replaced between MD and refinement mixed two potentials under one recorded identity with no message. The Torch adapters already rehash before every calculator and refuse this.

status read converged 2/9 and its verdict line was silent about 6 collapsed and 1 unresolved, or about a transient model failure, whose text was only in that event's result.json. A left-handed cell's 'SSNEB requires fully periodic right-handed cells' was found the same way. Now they are counted by status and message, up to five in the text and all under no_band in --json.

status showed converged 2/2 while four events sat in the queue, because it read only the phase, step and failure from state.json. With refine_after_md a whole run's events wait there, so converged x/y could read as everything done.

A resume with a changed package version or calculator source warned once in a job log and added a line to restart-environment.jsonl, and status stayed clean. A run whose calculator source was edited to double its energy and forces at step 340 read the same as one that was never touched.

candidates.csv gave a class's first-seen direction as every row's label, so broken C0-O1 could sit under formed C-O. The class number still merges both directions.

docs/exact-restart.md named a strict mode that does not exist and did not say that nothing tests a calculator's exactness, while the README's own xTB example leaves it after each pause or kill. The campaign docs called that a rounding-level difference; docs/models.md said a changed device continues when the campaign's device option is refused; and the README did not say check ignores residual forces.

Every exception in a search became a failed search with no limit, so a model that broke after the start relaxed wrote failed records until the job's time limit: 880 failed searches and 3.7 MB in 8 s with a toy calculator on real EON, and no reason anywhere but the attempt records. If the model raises or returns a non-finite value during a search, it is now evaluated once on that search's starting structure, which it evaluated before. If that fails too, the run exits 1 without recording the search, and a rerun repeats it with its saved seed. If that succeeds, the failure belonged to the geometry: the search is recorded as failed, with what the model raised, and the run goes on. There is no count or threshold. The call into the model records the failure itself, because EON's own code can report it as an ordinary ending. For the same reason a Ctrl-C inside a model call is raised again once EON returns, rather than written down as a failed search that changes every later record.

A step's result.json without its bonds, or a state.json from before global_frame, still dropped the whole trajectory's reactions. One lost bundle was counted twice. A copy with results but no database or candidates read as empty, and a trajectory with records but no state.json as not started. A last-error.json holding something other than a string crashed the whole report.

The observation count rounded down, though MD also observes the end of a last, shorter interval, so 25 steps at one per 10 were told they could not report what they then reported. A .con structure was told it has constraints, because ASE's reader attaches a FixAtoms that fixes nothing. A cell-less structure at pressure was told MD can run, and it fails at once. And validate read a structure twice per trajectory, 6.8 s for 2000 trajectories of 288 atoms against 0.36 s.

ASE sees through .gz, .bz2 and .xz, while the check read the compressed bytes as text, found no columns and told the user to add an element column the file already had.

The README's paragraph on commands for finished output keeps both additions: check reports a band's residual forces, and export-electrace lays a band out as VASP frame folders.

Austin243 added 26 commits November 6, 2026 14:19

Refuse eon.displace_centers on a push that ignores it

133425a

displace_centers applies only to a local push, and displace defaults to all, so a campaign setting only the centers pushed every atom without a word, and its exploration was not the restricted one it asked for.

Count events waiting to be refined in status

db6fc54

status showed converged 2/2 while four events sat in the queue, because it read only the phase, step and failure from state.json. With refine_after_md a whole run's events wait there, so converged x/y could read as everything done.

Label each scan row by its own direction

867b1a3

candidates.csv gave a class's first-seen direction as every row's label, so broken C0-O1 could sit under formed C-O. The class number still merges both directions.

Read a compressed dump for the LAMMPS type check

964c4d1

ASE sees through .gz, .bz2 and .xz, while the check read the compressed bytes as text, found no columns and told the user to add an element column the file already had.

Merge main: export a refined band for ElecTrace (#87)

ec6924c

The README's paragraph on commands for finished output keeps both additions: check reports a band's residual forces, and export-electrace lays a band out as VASP frame folders.

Navigation Menu

Repository navigation

Stop giving wrong or incomplete results without saying so - #86

Stop giving wrong or incomplete results without saying so#86
Austin243 wants to merge 26 commits into
mainfrom
fix/misleading-results

Austin243 commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Austin243 commented Nov 6, 2026

Summary

What to look at when merging

Left out on purpose

Testing

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant