feat: Add Magnetohydrodynamics (MHD) benchmark suite#9
Merged
Conversation
Reformatted math equations for better readability.
- Rename ArrayView/XView to ArrayView2D/ArrayView1D, drop ConstArrayView - Remove all _inplace wrappers and ConstArrayView overloads - Drop default constructor parameters (domain fixed to [0,1]) - Use constexpr coefficient table for SSP-RK3 substeps - Extract init_brio_wu_primitive() in main.cpp - Add cell center view (x) to SolverWorkspace, remove cell_centers() - Update tests to match new type names and required constructor args
Remove dt and t_final from SolverWorkspace constructor and members. Pass them directly to evolve_ssp_rk3(workspace, dt, t_final) instead. Constructor now takes (nx, gamma, bx) only.
…xing - Simplify initialize() signature: remove discontinuity_x parameter, hardcode 0.5 inside; remove unused constants from main.cpp - Fix cell center initialization to only fill interior cells (Lbx..Ubx) instead of all padded cells, matching the physical domain convention - Rename local dt/t_final to delt/tmax in main()
…0 cells Update default Nx constant and regenerate golden/reference CSV files. Improve plot styling with math mode labels and remove subplot titles.
Split the Brio-Wu reference solver and its test fixtures into shared assets, remove the JSON-based helper path, and make the solver grid size configurable
- cpp-full1d-01 exposes only evolve_ssp_rk3(...) as public solver entrypoint - HLLD and CLI remain provided, same as cpp-full1d-00 - Public C++ tests adapted to reduced scaffolding - Fixed hidden eval path in both 00 and 01 to use /eval_shared mount Co-authored-by: OpenCode Assistant <assistant@opencode.ai>
The eval test was using relative paths based on __file__ position which
breaks when tests run from /eval/tests/. Fixed to use absolute paths:
- WORKSPACE_ROOT = Path("/work/workspace")
- REFERENCE_CSV_PATH uses /eval_shared mount
This affects both cpp-full1d-00 and cpp-full1d-01 variants.
The workdir is mounted at /work, not /work/workspace. The original workspace subdirectory doesn't exist in the run workdir.
Owner
Author
|
Review @codex |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70f3fa1392
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+152
to
+153
| for (int ix = lbx - 1; ix <= ubx + 1; ++ix) { | ||
| ::hlld_flux_from_primitive(&up_l(ix, 0), &up_r(ix + 1, 0), bx, gamma, &flux(ix, 0)); |
There was a problem hiding this comment.
The flux loop iterates through ix <= ubx + 1 while calling up_r(ix + 1, 0), so on the last iteration it reads up_r(ubx + 2, 0) past the allocated Nx + 2*N_margin buffer (N_margin=1). This introduces undefined behavior in the shared reference solver (AddressSanitizer reports a heap-buffer-overflow), which can crash reference generation/tests and make benchmark outputs non-deterministic across builds.
Useful? React with 👍 / 👎.
The flux loop was iterating to ubx + 1 and accessing up_r(ix + 1), which read past the allocated Nx + 2*N_margin buffer on the last iteration. Changed loop bound from ubx + 1 to ubx to stay within allocated memory.
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
Adds a complete Magnetohydrodynamics (MHD) benchmark suite with 4 C++ tasks:
cpp-hlld-00: HLLD Riemann solver with detailed guidancecpp-hlld-01: HLLD variant with reduced guidancecpp-full1d-00: Full 1D ideal MHD solver (main+HLLD provided)cpp-full1d-01: Full 1D variant with minimal scaffoldingChanges
shared/eval/,mhd1d_shared.py)nx=200) against hidden referencesagents_default.tomlfor opencode config handling.gitignorefor build artifactsTesting
All tasks include:
1.0e-12Related
docs/toolchain.md: C++ headers (cxxopts, Catch2) already documented/work/basic_equations.mdand/work/hlld.md