Update nancheck to ensure exit using openmp #239
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
|
Since this conflicts with changes in #236, let's wait until that is merged before rebasing the changes here onto that. |
This commit addresses Issue 238 primarily in the NanCheck.cpp file, where the switch to the master thread (#pragma omp master) is placed AFTER the start of the "if" statement involving the "stop" variable, rather than before. I suspect that since the "stop" variable was altered under the "atomic write" OpenMP command, if any thread other than the master thread caught the Nan then the memory storing the "stop" variable would not be shared with master, and thus the if statement leading to the error trap would not be tripped if evaluated on the master thread. This change effectively moves the evaluation of that logic statement to each OpenMP thread individually. This commit also has an error-trip built into the ScalarField example, where in InitialScalarData I set the value of the extrinsic curvature K somewhere in the box to Nan. This version has a commented-out NanCheck right after the initial data as well, which can be used to debug the current issue (see end). I also added the specificPostTimeStep function to the ScalarField example, and moved the NanCheck that was called in specificAdvance to this function. PS. Another way to solve this problem could be to transfer the "stop" variable to the master thread in some way before the logic statement. This may address certain error output issues that are present in the current commit, meaning that if the error is caught on a non-master thread it is also printed by that same thread. We may need to look into this in the future.
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.
Fixes #238.
Also (just because I had this in my code already) implements the output of physical coordinates instead of integer ones if m_dx is passed by the user.
@mirenradia would you support moving the NanCheck to specificPostTimestep() instead of specifiAdvance()? I think this is a historic thing - I don't see that you gain much by exiting during a RK4 substep rather than at the end, and it means it runs a load more times. If so I would updated this in the Examples here.