Let fast_class enable classification on its own - #512
Open
krystophny wants to merge 1 commit into
Open
Conversation
Three call sites gate orbit classification: the bmin/bmax cache that separates trapped from passing, the dispatch to trace_orbit_with_classifiers, and the class_parts.dat writer. All three tested (ntcut > 0 .or. class_plot) and ignored fast_class, which is read only inside the classifier routine. So fast_class = .True. with tcut <= 0 and class_plot = .False. traced ordinary orbits, wrote no classification output, and left the flag inert, though it documents "quit immediately after fast classification". That combination is the only one that classifies without the Minkowski fractal cut: the cut fires at kt == ntcut, and the early exit in classification.f90 requires .not. class_plot. Without it, fast classification always costs either the fractal path or full-length traces. Extract the shared predicate as params.classification_enabled and include fast_class in it. Existing configurations are unaffected: both golden records set class_plot = .True., so the predicate keeps its value there.
This was referenced Aug 4, 2026
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
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.
Risk tier
A previously inert flag combination becomes live and now emits classification output.
Correctness contract
Intended behavior change
fast_class = .True.withtcut <= 0andclass_plot = .False.now enters theclassifier, writes
class_parts.dat, and takes the early exit atclassification.f90:370. Previously it traced ordinary orbits and wrote noclassification output, so the flag was silently a no-op.
Behavior that must not change
Every configuration that already set
tcut > 0orclass_plot = .True..classification_enabled()returns the same value for all of them, so thedispatch, the
bminmaxcache decision, and theclass_parts.datwriter areuntouched there. Both golden records (
classifier_fast,classifier_combined)set
class_plot = .True.and are unaffected.Coordinate / unit conventions
None touched.
Numerical invariants
None touched. No integrator, field, or classifier arithmetic is modified; only
the predicate that decides whether the classifier runs at all.
Tests added
bminmax_cache_cases.tsvgains afast_classcolumn plus three cases.0 0 F T Fis the discriminating one — it flips only becausefast_classnow enables classification. Read by both
test_bminmax_lifecycle(Fortran)and
test_bminmax_python(thepysimple._bminmaxmirror, updated to match).test_fast_class_driver.py, four namelist cases over theclassification matrix. Oracle is the classification output itself — a
classifying run owes a
class_parts.datwith one row per test particle, anon-classifying run owes none.
Verified the tests fail without the source change: 3 of 4 fail on reverted
sources, all 4 pass with the fix.
Golden-record impact
Both classifier golden records set
class_plot = .True., where the predicate'svalue is unchanged.
Failure modes considered
tcut > 0an unresolved orbit stopped atthe cut. Standalone
fast_classhas no such backstop, so it traces totrace_time. This trades "fractal runs on hard orbits" for "hard orbits runlong" — the right trade when the unresolved fraction is small. Callers who
want the cap keep setting
tcut.iclassstays 0 for unresolved orbits, which downstream consumers read astopology code 0, i.e. prompt loss. Pre-existing and not changed here, but it
becomes reachable in a new configuration, so it is worth stating: consumers
must separate genuine losses via
times_lost/exit_code.test_bminmax_pythonparses the same TSVand had to be updated with the Fortran test; caught by the suite.
Manual validation
make CONFIG=Fast, thenmake test TEST="bminmax|fast_class"— 4/4 pass.make test-fasthas 4 failures (test_chartmap_startmode1,test_spectre_sympl_volume,test_spectre_sympl_crossing,test_spectre_validation). All four fail identically on cleanmainat04b31afwith these changes stashed, so they are pre-existing and unrelated.