Skip to content

Migrate magnetic response table to Eigen3 AoS layout; consolidate nested-vector overloads as wrappers#490

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/migrate-magnetic-response-table
Draft

Migrate magnetic response table to Eigen3 AoS layout; consolidate nested-vector overloads as wrappers#490
Copilot wants to merge 4 commits intomainfrom
copilot/migrate-magnetic-response-table

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 24, 2026

ComputeMagneticFieldResponseTable and ComputeVectorPotentialCache used std::vector<std::vector<double>> as an intermediary between Eigen and ABSCAB, causing ~1M small heap allocations per call and strided memory access in the hot loops. Additionally, the same ABSCAB call logic was duplicated across two independent sets of overloads.

Memory layout: 3×N → N×3 (AoS)

RowMatrix3Xd is changed from Matrix<double, 3, Dynamic, RowMajor> to Matrix<double, Dynamic, 3, RowMajor>. In N×3 row-major, .data() is directly the flat AoS array [x0,y0,z0, x1,y1,z1, …] that ABSCAB expects, and per-point coordinate access (i, 0/1/2) is contiguous. Fixes the TODO(jurasic) bad loop order in MakeCylindricalGrid.

New flat-pointer AoS overloads (canonical implementations)

Added MagneticField / VectorPotential overloads for all filament types and MagneticConfiguration taking (int N, const double* eval_pos, double* result). These pass the Eigen .data() pointer directly to ABSCAB with zero copies.

Eliminated all STL intermediates in the hot path

ComputeMagneticFieldResponseTable and ComputeVectorPotentialCache no longer allocate cylindrical_grid_stl or magnetic_field_stl. The field buffer is allocated once outside the circuit loop and reused via setZero(). The MagneticConfiguration copy is also hoisted out of the loop.

// Before: per-circuit-per-filament nested-STL allocation + copy round-trip
std::vector<std::vector<double>> grid_stl(N);   // N heap allocs per circuit
std::vector<std::vector<double>> field_stl(N);  // N heap allocs per circuit
MagneticField(config, grid_stl, field_stl);
magnetic_field = ToEigenMatrix(field_stl).transpose();

// After: one Eigen N×3 buffer, passed directly to ABSCAB
magnetic_field.setZero();
MagneticField(config, N, cylindrical_grid.data(), magnetic_field.data());

Nested-vector overloads consolidated as thin wrappers

The six existing MagneticField / VectorPotential overloads taking vector<vector<double>> (used by LinkingCurrent and all tests) are now thin conversion wrappers that build a flat eval-positions buffer once, call the AoS implementation, and scatter the result back. This eliminates the duplicated ABSCAB setup logic while keeping the public API intact.

Result

test_bench_response_table_from_coils: ~1.32 s vs ~1.41 s baseline (~6% improvement). The remaining cost is the ABSCAB Biot-Savart integration itself (already OpenMP-parallelised internally).

Copilot AI and others added 4 commits April 27, 2026 10:45
… intermediate allocations

Agent-Logs-Url: https://github.com/proximafusion/vmecpp/sessions/65c3a9ff-885f-47f8-8eb5-df4cd04cdcfd

Co-authored-by: jurasic-pf <166746189+jurasic-pf@users.noreply.github.com>
Agent-Logs-Url: https://github.com/proximafusion/vmecpp/sessions/65c3a9ff-885f-47f8-8eb5-df4cd04cdcfd

Co-authored-by: jurasic-pf <166746189+jurasic-pf@users.noreply.github.com>
…mplementations

Agent-Logs-Url: https://github.com/proximafusion/vmecpp/sessions/de8e1d90-6465-4aa1-a851-a12a0616a754

Co-authored-by: jurasic-pf <166746189+jurasic-pf@users.noreply.github.com>
Co-authored-by: jurasic-pf <166746189+jurasic-pf@users.noreply.github.com>
@jurasic-pf jurasic-pf force-pushed the copilot/migrate-magnetic-response-table branch from 2ad49e4 to c6aeccb Compare April 27, 2026 08:54
Copy link
Copy Markdown
Collaborator


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Collaborator

@jons-pf jons-pf left a comment

Choose a reason for hiding this comment

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

LGTM, except that I would suggest to also migrate the magnetic field computation for InfiniteStraightFilament to accept the AoS data directly for consistency.

@jurasic-pf jurasic-pf marked this pull request as draft April 27, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants