Skip to content

Deprecate Banded Matrices in ForceFreeStates - #286

Merged
github-actions[bot] merged 7 commits into
developfrom
no_banded
Jun 19, 2026
Merged

Deprecate Banded Matrices in ForceFreeStates#286
github-actions[bot] merged 7 commits into
developfrom
no_banded

Conversation

@jhalpern30

Copy link
Copy Markdown
Collaborator

A long overdue removal of mband related terms within ForceFreeStates that clutters up the code. This PR closes#50.

Basically we decided banded matrices were untested and not applicable for perturbed equilibria, so having this functionality only added unnecessary complexity. Since mband = mpert - 1 in this case, this PR replaces this throughout the code and simplifies things where necessary.

Note that there is still some banded matrix code that exists in the Galerkin solver that I am not touching here. @matt-pharr can decide whether or not this stays since I don't know that part of the code

@OpenFUSIONToolkitOpenFUSIONToolkit deleted a comment from claudeBotJun 16, 2026
@jhalpern30

Copy link
Copy Markdown
CollaboratorAuthor

@copilot review this PR

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the deprecated/unused “banded matrix” plumbing from the ForceFreeStates stability pipeline (closing #50) by eliminating mband/delta_mband concepts and consistently treating the coupled operators as dense over the full mpert range. It also renames the FFT coefficient container field from mbandmmax to better reflect “highest retained mode”.

Changes:

  • Remove delta_mband from user configs and remove mband from ForceFreeStates internal/control structs and matrix-build paths, switching indexing/loop bounds to mpert.
  • Update metric/Fourier coefficient handling (Utilities.FourierCoefficients) and all call sites/tests/docs from mbandmmax.
  • Refresh examples, regression TOMLs, tests, and benchmark scripts to match the new API and parameter set.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
test/test_data/regression_solovev_kinetic_multi_n/gpec.tomlDrops delta_mband from regression config.
test/test_data/regression_solovev_kinetic_example/gpec.tomlDrops delta_mband from regression config.
test/test_data/regression_solovev_kinetic_calculated/gpec.tomlDrops delta_mband from regression config.
test/test_data/regression_solovev_ideal_example/gpec.tomlDrops delta_mband from regression config.
test/test_data/regression_solovev_ideal_example_multi_n/gpec.tomlDrops delta_mband from regression config.
test/runtests_utilities.jlUpdates assertions to use FourierCoefficients.mmax.
test/runtests_sing.jlUpdates FourFitVars construction to remove mband kwarg.
test/runtests_riccati.jlRemoves mband usage in setup and updates make_metric call signature.
test/runtests_parallel_integration.jlRemoves mband setup/usage and updates make_metric calls.
test/runtests_fouriertransforms.jlUpdates empty FourierCoefficients test to use mmax.
src/Utilities/FourierCoefficients.jlRenames mbandmmax and keeps Nyquist clamping logic.
src/PerturbedEquilibrium/FieldReconstruction.jlRemoves mband coupling limits and reindexes convolution buffers by mpert.
src/GeneralizedPerturbedEquilibrium.jlRemoves delta_mband/mband runtime logic and updates make_metric call + output fields.
src/ForceFreeStates/Sing.jlRemoves band-limited loop bounds; treats matrices as fully dense over mpert.
src/ForceFreeStates/Free.jlRemoves mband destructuring and reindexes jmat lookups by mpert.
src/ForceFreeStates/Fourfit.jlChanges make_metric API to (equil, mpert) and threads mpert through Fourier/matrix assembly.
src/ForceFreeStates/ForceFreeStatesStructs.jlRemoves mband from structs and adjusts FourFitVars storage sizing to mpert.
examples/Solovev_ideal_example/gpec.tomlDrops delta_mband from example config.
examples/Solovev_ideal_example_multi_n/single_n_2/gpec.tomlDrops delta_mband from example config.
examples/Solovev_ideal_example_multi_n/single_n_1/gpec.tomlDrops delta_mband from example config.
examples/Solovev_ideal_example_multi_n/gpec.tomlDrops delta_mband from example config.
examples/Solovev_ideal_example_3D/gpec.tomlDrops delta_mband from example config.
examples/LAR_epsilon_scan/gpec.tomlDrops delta_mband from example config.
examples/LAR_beta_scan/gpec.tomlDrops delta_mband from example config.
examples/DIIID-like_ideal_example/gpec.tomlDrops delta_mband from example config.
examples/a10_kinetic_example/gpec.tomlDrops delta_mband from example config.
docs/src/utilities.mdUpdates documentation to use fc.mmax.
docs/src/stability.mdUpdates stability docs to remove mband and use new make_metric call.
benchmarks/benchmark_threads.jlUpdates benchmark setup to remove mband and use (equil, intr.mpert) metric call.
benchmarks/benchmark_riccati_der.jlUpdates benchmark setup to remove mband and use (equil, intr.mpert) metric call.
benchmarks/benchmark_diiid_kinetic_stability.jlRemoves delta_mband from generated TOML and updates formatting.
benchmarks/benchmark_diiid_ideal_ntv_torque.jlRemoves delta_mband from generated TOML and updates make_metric call.
benchmarks/benchmark_delta_prime_methods.jlUpdates benchmark setup to remove mband and use (equil, intr.mpert) metric call.

Comment threadsrc/ForceFreeStates/ForceFreeStatesStructs.jl
Comment threadsrc/ForceFreeStates/Fourfit.jl
Comment threadtest/runtests_riccati.jl
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

CopilotAI commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@copilot review this PR

Addressed a high-impact PR regression in commit df57afa: legacy delta_mband in [ForceFreeStates] no longer crashes startup. I added a compatibility parser path that drops delta_mband with a deprecation warning, plus a targeted test in runtests_utilities.jl to lock this behavior.

Co-authored-by: jhalpern30 <57007344+jhalpern30@users.noreply.github.com>
@jhalpern30
jhalpern30 requested a review from logan-ncJune 16, 2026 17:48
@jhalpern30

Copy link
Copy Markdown
CollaboratorAuthor

Regression harness (first time running this, this is very cool)

Regression Report: solovev_multi_n

Ref 1: develop @ bb3c503 (2026-06-14)
Ref 2: local @ local (2026-06-16)
Summary: 15 unchanged

Regression Report: diiid_n1

Ref 1: develop @ bb3c503 (2026-06-14)
Ref 2: local @ local (2026-06-16)
Summary: 30 unchanged

Regression Report: solovev_kinetic_calculated

Ref 1: develop @ bb3c503 (2026-06-14)
Ref 2: local @ local (2026-06-16)
Summary: 15 unchanged

Regression Report: solovev_n1

Ref 1: develop @ bb3c503 (2026-06-14)
Ref 2: local @ local (2026-06-16)
Summary: 21 unchanged

Regression Report: ggj_reference

Ref 1: develop @ bb3c503 (2026-06-14)
Ref 2: local @ local (2026-06-16)
Summary: 4 unchanged

@matt-pharr

Copy link
Copy Markdown
Collaborator

I support getting rid of the banded matrix functionality.

… Nyquist msg)
Legacy gpec.toml files still setting delta_mband/mband under [ForceFreeStates]
previously crashed at startup with an unknown-keyword error when the table was
splatted into ForceFreeStatesControl. Add _drop_deprecated_ffs_keys! to strip
deprecated keys with a deprecation warning before the splat.
Also make the FourierCoefficients Nyquist assert report the actual mmax, Nyquist
limit, and ntheta instead of a bare message.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@logan-nclogan-nc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great!

@logan-nclogan-nc self-assigned this Jun 19, 2026
@logan-nclogan-nc added refactor Behavior-preserving restructuring auto-merge labels Jun 19, 2026
@github-actions
github-actionsBot enabled auto-merge June 19, 2026 03:55
@github-actions
github-actionsBot merged commit 2f640c5 into developJun 19, 2026
9 checks passed
@github-actions
github-actionsBot deleted the no_banded branch June 19, 2026 04:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactorBehavior-preserving restructuring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Either making a Julia form of Banded Matrices work or ditching it in the code entirely

5 participants

@jhalpern30@matt-pharr@logan-nc