Remove restriction on CUDA that amr.blocking_factor be a multiple of 8 #1049
Conversation
This file contains 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
Though this is recommended for performance reasons, it was previously required due to correctness reasons because of the use of an AMReX Fortran function, amrex_filccn. As noted in AMReX-Astro#1048, this function is no longer required due to recent changes in Castro (problems overriding bc_fill_nd.F90 or bc_ext_fill_nd.F90 do not need to provide an initial fill of the ghost zone data before implementing their specific boundary conditions; this is now done for you). Calling this function may now result in race conditions and correctness issues in the CUDA build, so it should be removed from any problem setups. (AMReX-Astro#1049)
zingale
approved these changes
Jun 21, 2020
zingale
pushed a commit
that referenced
this pull request
Oct 30, 2020
We had rules in the build system that capped the number of registers per thread used in the CUDA build for the boundary condition routines. This allowed us to be consistent with similar logic I had inserted in AMReX in the Fortran filcc. However, we no longer use that boundary condition interface, so the special handling we have in the build system is no longer necessary (#819, #1048, #1049). These CUDA build rules are a holdover from that time and have been obsolete for a while.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR summary
Though using a blocking factor >= 8 is recommended for performance reasons, it was previously required due to correctness reasons because of the use of an AMReX Fortran function, amrex_filccn. As noted in #1048, this function is no longer required due to recent changes in Castro (problems overriding bc_fill_nd.F90 or bc_ext_fill_nd.F90 do not need to provide an initial fill of the ghost zone data before implementing their specific boundary conditions; this is now done for them). Calling this function may now result in race conditions and correctness issues in the CUDA build, so it should be removed from any problem setups.
PR checklist
CHANGESfile has been updated, if appropriate