Uh oh!
There was an error while loading. Please reload this page.
TEST - NEW FEATURE - Coverage for Vacuum in-place response, surface extraction, field - #260
Conversation
…xtraction, field Addresses #60. Adds test/runtests_vacuum_coverage.jl covering three previously-untested public Vacuum entry points, kept separate from the large runtests_vacuum.jl to avoid diff churn: - compute_vacuum_response!: verifies the in-place entry populates caller-owned, duck-typed (NamedTuple) storage identically to the allocating wrapper, for nowall and conformal walls; green_only suppression is tested with a non-green_only negative control so the zeroed wv assertion is load-bearing. - extract_plasma_surface_at_psi: physical sanity on a self-contained analytic Solovev equilibrium (R>0, surface brackets the axis, extent grows with psi). - compute_vacuum_field: DISCOVERED LATENT BUG — the function is unreachable, gated by two independent defects (undefined vaccal! at Vacuum.jl:317, and _pickup_field reading the nonexistent inputs.n at Field.jl:94). Documented with @test_broken + tripwire assertions; source intentionally NOT modified. No source modified; full default suite passes (exit 0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jhalpern30
left a comment
There was a problem hiding this comment.
I think this more or less can just become a small PR adding the compute_vacuum_response and extract_surface_at_psi unit tests to the existing runtests_vacuum..jl file.
| else | ||
| include("./runtests_utilities.jl") | ||
| include("./runtests_vacuum.jl") | ||
| include("./runtests_vacuum_coverage.jl") |
There was a problem hiding this comment.
this shouldn't be a new file
| Eq = GeneralizedPerturbedEquilibrium.Equilibrium | ||
| # A circular plasma boundary; mtheta_eq points define the boundary spline. | ||
| _make_inputs(; mtheta=64, mtheta_eq=17, mpert=2, mlow=1, nlow=1, npert=1) = VacuumInput( |
There was a problem hiding this comment.
a similar function is already defined in runtests_vacuum.jl - just reuse
| mtheta=mtheta | ||
| ) | ||
| @testset "compute_vacuum_response! matches allocating wrapper" begin |
There was a problem hiding this comment.
merge into the existing compute_vacuum_response testset that already exists
| end | ||
| end | ||
| @testset "compute_vacuum_response! green_only suppresses the response matrix" begin |
There was a problem hiding this comment.
I am going to deprecate this in #190 - I ran tests and it saves like <1% of the computation time and no memory, so no need for the added complexity. I'd just remove this test in anticipation of that unless you have a strong reason for me to keep the functionality
| @test extent(r_out, z_out) > extent(r_in, z_in) | ||
| end | ||
| @testset "compute_vacuum_field — LATENT BUG (unreachable: vaccal! + inputs.n)" begin |
There was a problem hiding this comment.
This seems like a weird unit test to add for placeholder code that isn't working yet. Why not just add in an @error to the beginning of the compute_vacuum_field call? I can see it being annoying when someone implements that and pushes the PR only for their unit tests to break and they have to track down this test. I think the error statement does the same purpose and puts all of the code in the same place
logan-nc
commented
Jun 5, 2026
yeah, do it please. My objective was to provide the impulse for folks to close out low hanging fruit issues. I am not attached to the AI attempt to do so (was trying to spend as many tokens as possible yesterday). I just want he issues closed |
Address PR #260 review: drop the standalone runtests_vacuum_coverage.jl and merge the two useful tests into the existing suite, reusing the compute_vacuum_response testset's _make_inputs helper. - compute_vacuum_response! in-place vs. allocating-wrapper equivalence (nowall + conformal), added inside the existing testset. - extract_plasma_surface_at_psi physical sanity on a Solovev equilibrium. Removed the green_only test (functionality being deprecated in #190) and the compute_vacuum_field latent-bug tripwire (placeholder code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
logan-nc
commented
Jun 11, 2026
@jhalpern30 how about this trimmed version? |
jhalpern30
commented
Jun 11, 2026
Beautiful. Will merge after tests pass |
Uh oh!
There was an error while loading. Please reload this page.
Address PR #260 review: drop the standalone runtests_vacuum_coverage.jl and merge the two useful tests into the existing suite, reusing the compute_vacuum_response testset's _make_inputs helper. - compute_vacuum_response! in-place vs. allocating-wrapper equivalence (nowall + conformal), added inside the existing testset. - extract_plasma_surface_at_psi physical sanity on a Solovev equilibrium. Removed the green_only test (functionality being deprecated in #190) and the compute_vacuum_field latent-bug tripwire (placeholder code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Closes part of #60 (unit-test coverage). Adds coverage for two public Vacuum entry points not exercised by
runtests_vacuum.jl, folded directly into the existing suite.compute_vacuum_response!(in-place) — asserts it populates caller-owned duck-typed (NamedTuple) storage identically to the allocatingcompute_vacuum_responsewrapper, fornowallandconformalwalls.extract_plasma_surface_at_psi— physical sanity on a self-contained analytic Solovev equilibrium: correct array lengths, all-finiter/z/ν,R > 0, the surface brackets the magnetic axis inR, and minor-radius extent grows monotonically from ψ=0.2 → 0.8.Both tests reuse the existing
_make_inputshelper and live inside the relevant testsets inruntests_vacuum.jl— no new test file.Files changed
test/runtests_vacuum.jltest/runtests.jltest/runtests_vacuum_coverage.jlVerification
Tests-only, no source modified — regression golden values unaffected.
🤖 Generated with Claude Code