find_bminmax: initialize cache in active tracing coordinates#348
Merged
Conversation
The lazy init inside get_bminmax was triggered after the coordinate switch to canonical/Boozer. find_bminmax scans (theta, phi) to locate field extrema, which is only meaningful when theta/phi are geometric VMEC angles. Make the init explicit and run it before init_magfie(isw) so the scan stays in VMEC space. Gate the call on num_surf /= 1: compute_pitch_angle_params is the only caller of get_bminmax, and it only reads from the cached arrays when num_surf /= 1. The default num_surf=1 case was the dominant golden record cost, so the gate keeps runtime flat for everything that does not actually use the cache. While here, fix the matching condition in compute_pitch_angle_params: num_surf > 1 missed the num_surf == 0 (volume sampling) case that also needs per-particle bmin/bmax.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
This was referenced May 21, 2026
krystophny
added a commit
that referenced
this pull request
May 22, 2026
…ion (#323) ## Summary Three logical commits that together reproduce the orbit-classification figures of the **2023** JPP paper (Albert, Buchholz, Kasilov, Kernbichler, Rath, *J. Plasma Phys.* 89(3), 955890301, [doi:10.1017/S0022377823000351](https://doi.org/10.1017/S0022377823000351)) on current `main` and fix two classification regressions blocking it. 1. `fix(classification): allow fast_class + tcut together` - `fast_class` no longer marks regular orbits as lost. Previously `ierr = ierr_cot` propagated the classifier completion status to the integration error flag and every classified orbit was reported as lost (confined fraction ~ 0.57 instead of 0.94 for QI s=0.3). The fix sets `regular = .True.` for regular J_par orbits under `fast_class .and. .not. class_plot`; `class_plot` mode keeps the old ntcut-termination so classification output still gets written. - `read_config` no longer rejects `fast_class .and. tcut > 0`. The paper recipe combines both flags. - A `classifier_combined` golden record exercises both together. 2. `feat(examples/classification): JPP 2023 paper reproduction recipe` - 12 input files (`q{a,h,i}_{volume,s03,s06,fig8}.in`) covering Figs 5-11 of the paper. - `plot_paper_results.py` produces fig5-7 losses, fig8 dashboard, and volume_classification + volume_topology (s, J_perp) maps. - `run_all.sh` downloads the wout files from the companion repo and runs all 12 cases under `/tmp/simple_classification`. - Removes the legacy `plot_classification.py`, `postprocess_class.f90`, `simple.in`, and example `README.md` superseded by the new script set. 3. `test(magfie): skip orbit chartmap comparison plot when matplotlib unavailable` - Plot test no longer ImportError-crashes when optional Python deps are missing; it now exits 0 with a `Skipping plot test:` message. Companion repository with equilibrium files, plot-essential run outputs, and generated figures: https://github.com/itpplasma/proxima-simple-classification ## Verification ### Full `make test-fast` locally (100% pass) ``` $ make test-fast ... 100% tests passed, 0 tests failed out of 44 Total Test time (real) = 343.46 sec ``` Failing test before the fix (`test_bminmax_driver`, case `bminmax_classifier_num_surf_zero`) now passes; was caused by an earlier branch revision that bumped `num_surf` for `startmode=1 + num_surf=0`. The volume configs now use `startmode = 5` (the standard volume-sampling entry) instead, which keeps the bminmax cache semantics unchanged from #348. ### Confined fractions match paper recipe (volume runs, 200k particles) QA, QH, QI volume `(s, J_perp)` classification maps regenerated with this branch are at: https://github.com/itpplasma/proxima-simple-classification/tree/main/plots Single-surface loss curves (Figs 5-7) and dashboard (Fig 8) are being regenerated this week and will land in the same `plots/` directory. ## Test plan - [x] `make test-fast` passes 100% (44/44) locally - [x] `test_bminmax_driver` (which was failing on prior branch state) passes after switching volume configs to `startmode=5` - [x] `classifier_combined` golden record covers `fast_class + tcut` - [x] Volume `(s, J_perp)` plots reproduce the paper's QI, QH, QA maps with both J_par and ideal-topology classifier columns - [ ] Loss curves (Fig 5-7) and dashboard (Fig 8) regenerated with `multharm=7` (in flight, ~4-6 days serial)
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.
Summary
Verification