Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composition verification for external sources #443

Merged

Conversation

dmontgomeryNREL
Copy link
Contributor

@dmontgomeryNREL dmontgomeryNREL commented Nov 28, 2024

This PR adds a test to the EB_ODEQty regression test, namely composition-test-2d.inp, which verifies the implementation of user-defined external sources for density $\rho$ and species mass fractions $Y_m$. The initial mass fractions are set to $Y_{CO2} = 0.15$, $Y_{N2} = 0.25$, and $Y_{AR} = 0.6$.

After $t_s$ seconds, CO2 is added at a rate $S_{ext,\rho Y_{CO2}}$, which also causes the density to increase at a rate of $S_{ext,\rho} = S_{ext,\rho Y_{CO2}}$.

The results can be plotted against the analytical solutions in plot-comps.py (see figure below) and tested in the CI via test.py, where errors must be less than a specified maximum error threshold. This was done by adding add_test_rt to CMakeLists, which runs any python script called test.py in the specified case directory.

Additionally, this PR fixes a minor bug in problem_modify_ext_sources, where time is passed in via a_timestamp_old instead of a_timestamp_new due to the explicit treatment of the external source terms.

composition-test

@dmontgomeryNREL
Copy link
Contributor Author

Documenting the problem with ubuntu-24.04 EB-ON CI test:
The new test.py test for EB_ODEQty/composition-test-2d.inp passes during the macOS-latest EB-ON test, but fails during the ubuntu-24.04 EB-ON test. As shown in commit #c7a863a, the macOS and ubuntu tests run the associated PeleLMeX executable with four processors. The ubuntu test appears to be running the PeleLMeX executable four times while the macOS test runs the test in parallel. The temporals/tempExtremas data for each test are included in the images below.

Data read in during macOS test:
macos-test

Data read in during the ubuntu test (4 processors each adding to tempExtremas):
ubuntu-test

Tests/CMakeLists.txt Outdated Show resolved Hide resolved
@jrood-nrel
Copy link
Contributor

Let's see if MPI works better if we step ubuntu back #446 until I figure what happened to MPI in the latest image.

@jrood-nrel
Copy link
Contributor

So we can use ubuntu-24.04 which is also now ubuntu-latest on everything except for when we run with MPI. Looks like we need ubuntu-22.04 to run with MPI. I'm trying to figure out what happened to MPI on the new image.

@jrood-nrel jrood-nrel self-requested a review December 12, 2024 17:43
@dmontgomeryNREL
Copy link
Contributor Author

@jrood-nrel, any idea why there are expected tests for CPU-CMake ubuntu-24.04 in the testing queue?

@jrood-nrel
Copy link
Contributor

Those have to be updated after this is merged. It is because the required tests were renamed.

Copy link
Collaborator

@baperry2 baperry2 left a comment

Choose a reason for hiding this comment

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

Looks good, just the minor change for ignore-unused.

ext_src_arr[box_no](i, j, k, FIRSTODE + n) += src;
}
// Ignore time as it is only used if composition_test = 1
amrex::ignore_unused(time);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This ignore unused and the one below shouldn't be needed. ignore-Unused should only be needed when certain variables are excluded by compile time macros, not runtime conditionals

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying. I made the update and merged the most recent branch.

Exec/RegTests/EB_ODEQty/eb-odeqty-3d.inp Outdated Show resolved Hide resolved
@baperry2 baperry2 merged commit 652f0ea into AMReX-Combustion:development Dec 17, 2024
24 checks passed
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