(invalid) define env_isDistributed()#764
Open
TysonRayJones wants to merge 1 commit into
Open
Conversation
This was an attempt to avoid consulting comm_isInit() internally, which only indicates MPI is active, and not whether QuEST is allowed to use it! (QuEST itself may be launched non-distributed). It was already an ugly attempt because of the nuisance of not exposing env_isDistributed in a header... But it is in fact a totally inadequate solution due to a single function: gpu_areAnyNodesBoundToSameGpu()! This is the only function in all of QuEST which needs to perform an MPI communication before the validation within initQuESTEnv has been completed (indeed, it's invoked by validation), in scenarios where the validation is not currently failing. Grr!
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This was an attempt to avoid consulting
comm_isInit()internally, which only indicates MPI is active, and not whether QuEST is allowed to use it! (QuEST itself may be launched non-distributed within a user-owned MPI environment). This was to relax the current restriction highlighted here.This was already an ugly attempted solution because of the nuisance of not being able to expose internal functioj
env_isDistributed()within the user-facingenvironment.hheader...It is, in fact a totally inadequate / non-functioning solution! This is due entirely to a single function;
gpu_areAnyNodesBoundToSameGpu(). This is the only function in all of QuEST which needs to perform an MPI communication before the validation withininitQuESTEnvhas been completed (indeed, it's invoked by validation), in scenarios where the validation is not in the process of failing. This precludes us from callingenv_isDistributed()within it, since theQuESTEnvis stillnullptr, and the "in-process"initQuESTEnvcall has nowhere to indicate that it intends to distribute QuEST. More info in thegpu_config.cppdiff.Consistent with my typical luck, this function is defined in
gpu_config.cpp, which was shown at the very bottom of my IDE's search results for files containingcomm_isInit(). Ergo, it was the very last function to be updated in this refactor, maximally prolonging my ordeal.I've made this PR anyway for reference while re-attempting a solution. Thankfully, a cleaner and more robust design exists (consulting of
mpiCom == NULLfrom withincomm_config.cpp)