Skip to content

Read 2D chemistry probe species after locating the probe - #2125

Open
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:wave1/2d-chem-probe-indices
Open

sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:wave1/2d-chem-probe-indices

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

What

In the 2D branch of s_write_probe_files the species partial densities
were read from q_cons_vf(j-2, k-2, l) before j, k and l were computed
for the current probe and outside the ownership check, so they used the
previous probe's cell or, for the first probe, uninitialized indices.
A gfortran debug run of a 2D chemistry case with probes aborted on an
out-of-bounds index.

Move the read inside the ownership branch after the indices are set,
as in the 1D and 3D branches.

Verification

Testing. Built with GPU offload (OpenMP, AMD) and ran the full test suite with this fix and seven other independent fixes applied together: 729 tests in 4 shards, 0 failures (two shards on MI300A, two on MI250X).

Prepared with Claude Code (AI-assisted); I reviewed the change and ran the tests.

In the 2D branch of s_write_probe_files the species partial densities
were read from q_cons_vf(j-2, k-2, l) before j, k and l were computed
for the current probe and outside the ownership check, so they used the
previous probe's cell or, for the first probe, uninitialized indices.
A gfortran debug run of a 2D chemistry case with probes aborted on an
out-of-bounds index.

Move the read inside the ownership branch after the indices are set,
as in the 1D and 3D branches.
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Claude Code Review

Head SHA: 096a3a4

Files changed:

  • 1
  • src/simulation/m_data_output.fpp

Findings:
No issues found. Checked in step 4: src/simulation/m_data_output.fpp. The 2D probe branch read rhoYks with stale j,k before they were set by minloc. The diff moves the read after j,k are assigned and inside the ownership check, which fixes this. This matches the 1D and 3D branches.

@sbryngelson
sbryngelson marked this pull request as ready for review November 3, 2026 13:53
@sbryngelson
sbryngelson requested a review from a team as a code owner November 3, 2026 13:53
@sbryngelson
sbryngelson requested review from danieljvickers and removed request for a team November 3, 2026 13:53
@codecov

codecov Bot commented Nov 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.90%. Comparing base (a03848a) to head (096a3a4).

Files with missing lines Patch % Lines
src/simulation/m_data_output.fpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2125   +/-   ##
=======================================
  Coverage   61.90%   61.90%           
=======================================
  Files          86       86           
  Lines       22948    22948           
  Branches     3386     3386           
=======================================
  Hits        14205    14205           
  Misses       6229     6229           
  Partials     2514     2514           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant