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

Some improvements and testing for the EB covered state #704

Merged
merged 14 commits into from
Oct 24, 2023

Conversation

baperry2
Copy link
Contributor

This closes #703, with the goal of ensuring that covered cells never affect fluid cell values:

  • A change in behavior for all simulations - if a NaN is detected in the state after a timestep, the simulation now stops
  • For all EB stencils, if a value in the stencil is zero, that element of the stencil is ignored rather than multiplying it by the value in the cell and adding it into the gradient being computed. This should not effect the solution, but is done so that if the value is NaN in a covered cell, it does not propagate into the domain.
  • A new function is added to abort if the stencil for gradients at the boundary tries to access covered cells (can happen for the quadratic option on coarse grids ebd.boundary_grad_stencil_type = 0)
  • the function zero_in_body has been deleted because it was not used anywhere
  • A new option to set the body state to 0 for all variables for debugging (to ensure this does not impact fluid cells)
  • The eb-c3 test is modified to set the body state to 0. This is done to prevent any future changes that would cause data from covered cells to be accessed during the fluid cell updates. Had to make a few changes to Tests/CMakeLists.txt because this test can't run with AMReX FPE trapping.

Copy link
Contributor

@hsitaram hsitaram 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! thank you, Bruce.

@baperry2
Copy link
Contributor Author

The end of timestep NaN check works at least on AMD and Nvidia GPUs (Frontier and Eagle).

On GPU, the abort for invalid stencils only works in debug mode or with USE_ASSERTION=TRUE.

Did not notice a change in performance.

@@ -55,11 +55,11 @@ macro(setup_test)
file(COPY ${CURRENT_TEST_SOURCE_DIR}/${TEST_NAME}.inp DESTINATION "${CURRENT_TEST_BINARY_DIR}/")
file(COPY ${TEST_FILES} DESTINATION "${CURRENT_TEST_BINARY_DIR}/")
# Set some default runtime options for all tests
set(RUNTIME_OPTIONS "amr.plot_file=plt amr.checkpoint_files_output=0 amr.plot_files_output=1 amrex.the_arena_is_managed=0")
set(RUNTIME_OPTIONS_AlL "amr.plot_file=plt amr.checkpoint_files_output=0 amr.plot_files_output=1 amrex.the_arena_is_managed=0")
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a typo here.

@@ -113,6 +113,14 @@ function(add_test_rf TEST_NAME TEST_EXE_DIR)
set_tests_properties(${TEST_NAME} PROPERTIES WILL_FAIL TRUE)
endfunction(add_test_rf)

# Regression tests no FPE trapping ever
Copy link
Contributor

Choose a reason for hiding this comment

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

This test won't have these options amr.plot_file=plt amr.checkpoint_files_output=0 amr.plot_files_output=1 amrex.the_arena_is_managed=0, which I think we would want to enforce.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I'm confused. It just has the same options as the other tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

You could leave the RUNTIME_OPTIONS as is and just add amrex.signal_handling=0 to this test category.

Source/PeleC.cpp Outdated Show resolved Hide resolved
Source/EB.cpp Outdated Show resolved Hide resolved
@baperry2 baperry2 enabled auto-merge (squash) October 23, 2023 22:47
@baperry2 baperry2 merged commit acc0f7e into AMReX-Combustion:development Oct 24, 2023
14 checks passed
@baperry2 baperry2 deleted the eb-state branch October 24, 2023 14:17
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.

Ensure unnecessary operations are not carried out in covered regions
4 participants