Skip to content

Let fast_class enable classification on its own - #512

Open
krystophny wants to merge 1 commit into
mainfrom
fix/fast-class-standalone-dispatch
Open

Let fast_class enable classification on its own#512
krystophny wants to merge 1 commit into
mainfrom
fix/fast-class-standalone-dispatch

Conversation

@krystophny

Copy link
Copy Markdown
Member

Risk tier

  • T3: physics, output behavior, coordinate convention

A previously inert flag combination becomes live and now emits classification output.

Correctness contract

Intended behavior change

fast_class = .True. with tcut <= 0 and class_plot = .False. now enters the
classifier, writes class_parts.dat, and takes the early exit at
classification.f90:370. Previously it traced ordinary orbits and wrote no
classification output, so the flag was silently a no-op.

Behavior that must not change

Every configuration that already set tcut > 0 or class_plot = .True..
classification_enabled() returns the same value for all of them, so the
dispatch, the bminmax cache decision, and the class_parts.dat writer are
untouched 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

  • unit: bminmax_cache_cases.tsv gains a fast_class column plus three cases.
    0 0 F T F is the discriminating one — it flips only because fast_class
    now enables classification. Read by both test_bminmax_lifecycle (Fortran)
    and test_bminmax_python (the pysimple._bminmax mirror, updated to match).
  • integration: test_fast_class_driver.py, four namelist cases over the
    classification matrix. Oracle is the classification output itself — a
    classifying run owes a class_parts.dat with one row per test particle, a
    non-classifying run owes none.
  • system: none.
  • golden record: 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

  • unchanged

Both classifier golden records set class_plot = .True., where the predicate's
value is unchanged.

Failure modes considered

  • Orbits that never resolve. With tcut > 0 an unresolved orbit stopped at
    the cut. Standalone fast_class has no such backstop, so it traces to
    trace_time. This trades "fractal runs on hard orbits" for "hard orbits run
    long" — the right trade when the unresolved fraction is small. Callers who
    want the cap keep setting tcut.
  • iclass stays 0 for unresolved orbits, which downstream consumers read as
    topology 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.
  • Second reader of the case table. test_bminmax_python parses the same TSV
    and had to be updated with the Fortran test; caught by the suite.

Manual validation

make CONFIG=Fast, then make test TEST="bminmax|fast_class" — 4/4 pass.

make test-fast has 4 failures (test_chartmap_startmode1,
test_spectre_sympl_volume, test_spectre_sympl_crossing,
test_spectre_validation). All four fail identically on clean main at
04b31af with these changes stashed, so they are pre-existing and unrelated.

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.

@slopqueue slopqueue Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review verdict: Comment

See review above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S review size up to 100 changed lines tier/T3 physics or output behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant