Skip to content

run fcompare in CI#635

Merged
baperry2 merged 12 commits intoAMReX-Combustion:developmentfrom
baperry2:ci-test-ref
Mar 6, 2026
Merged

run fcompare in CI#635
baperry2 merged 12 commits intoAMReX-Combustion:developmentfrom
baperry2:ci-test-ref

Conversation

@baperry2
Copy link
Copy Markdown
Collaborator

@baperry2 baperry2 commented Mar 4, 2026

Build a reference version of PeleLMeX and compare results for RegTests during CI. Mirrors AMReX-Combustion/PeleC#917 and what is done for the UnitTests.

@baperry2 baperry2 marked this pull request as ready for review March 5, 2026 23:57
@baperry2 baperry2 enabled auto-merge (squash) March 5, 2026 23:57
@baperry2
Copy link
Copy Markdown
Collaborator Author

baperry2 commented Mar 6, 2026

Confirmed that this PR does catch intentional bugs (which have now been reverted): https://github.com/AMReX-Combustion/PeleLMeX/actions/runs/22740480520/job/65952674740

unitTest should still be failing because I added additional output variables that will not be in the output of the reference version (but all existing variables should now match with the intentional bugs reverted).

@baperry2 baperry2 requested review from Copilot and jrood-nrel March 6, 2026 00:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Build a reference version of PeleLMeX in CI and compare regression-test outputs against a saved “gold” reference, aligning behavior with existing UnitTest comparisons.

Changes:

  • Add CI support to optionally save golds, build a reference revision, and run comparison testing.
  • Tighten UnitTest plotfile comparison to fail when expected fields are missing.
  • Adjust projection update logic and extend UnitTest evaluate variables to support comparisons.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Source/PeleLMeX_Projection.cpp Changes which state/gp MultiFabs are used during the velocity projection update.
Exec/UnitTests/DodecaneLu/inputs.3d Adds additional derived/evaluated variables to the UnitTest output used for comparisons.
Docs/sphinx/manual/Troubleshooting.rst Fixes a spelling issue in troubleshooting documentation.
.github/workflows/unitTest.yml Makes fcompare fail if not all requested fields are found in plotfiles.
.github/workflows/ci.yml Adds optional “reference build + compare” flow and configuration to save golds.
Comments suppressed due to low confidence (1)

Source/PeleLMeX_Projection.cpp:1

  • These kernels iterate over ldataNew_p->state but write velocity using state_old_ma[box_no], which updates the old state arrays instead of the new state (and also risks mismatched FAB indexing if layouts ever diverge). Use the arrays from ldataNew_p->state for vel when applying the projection update, and ensure the gp_* array you apply corresponds to the same time/state you intend to update.
#include <PeleLMeX.H>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@baperry2 baperry2 disabled auto-merge March 6, 2026 18:39
@baperry2 baperry2 merged commit 218ff0e into AMReX-Combustion:development Mar 6, 2026
59 of 61 checks passed
@baperry2 baperry2 deleted the ci-test-ref branch March 6, 2026 18:39
@baperry2 baperry2 mentioned this pull request Mar 9, 2026
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