Fix NaNs in the code. Produces expected results.

[Pranay-Reddy-Kommera]

This pull request includes the suggestions made by Raghu in the code to fix the NaNs in the results.

Please, kindly close PR#9, #13 and #14. We do not need those changes to be included in the code.

[mgduda]

@Pranay-Reddy-Kommera I think you should have permission to close PR #9 #13 and #14 . When doing so, it may be good to leave a comment there explaining why the PRs are no longer needed (referencing this PR as appropriate).

[Pranay-Reddy-Kommera]

Sure. Will do it.

[Pranay-Reddy-Kommera]

@mgduda I have closed PR #9 , #13 and #14 . Please review PR #15 and #16 and, approve and merge if they are no concerns regarding the code changes.

[mgduda]

@Pranay-Reddy-Kommera It looks like there are two commits in this PR, the first of which is a merge to your atmosphere/gpu branch. Generally, I think we would like the history of everyone's atmosphere/gpu branch to match, since this is the basis for all of our work, and rather than making a PR from Raghu's atmosphere/gpu branch to yours, you can simply push Raghu's branch to yours to update. If you'd like, for now, I can force-push Raghu's atmosphere/gpu branch to your fork, then rebase the branch in this PR onto the resulting atmosphere/gpu branch.

You can imagine how confusing the history of a branch can become if that branch's history includes many repeated merges between itself and the same branch on someone else's fork. This is why we prefer to update branches with the same name (e.g., atmosphere/gpu) via a simple push (which should result in a fast-forward), rather than through PRs. Does this make sense?

[mgduda]

@Pranay-Reddy-Kommera The code changes themselves look fine, but can you update the commit message as well as the first comment in this PR (which will be used as the merge commit message) to explain the changes in more detail. Even being somewhat familiar with what's going on, I still don't understand exactly why we are changing some mpas_pool_get_array_gpu calls to mpas_pool_get_array calls. How does this fix NaNs in the code, exactly? Why is it these specific calls that are being modified and not others?

[Pranay-Reddy-Kommera]

They are two subroutines which are not being executed on GPU, so in that case if we have mpas_pool_get_array_gpu calls in that subroutines, we are copying/updating data on GPU which is resulting in asynchronous data between CPU and GPU during the time of OpenAC execution. That is the reason why we changed calls to mpas_pool_get_array in subroutines where no openacc code is present.

[Pranay-Reddy-Kommera]

@mgduda I understood what you are saying about updating my fork with raghu's code and then working on it. I will update my fork and then make changes to it and will create pull requests then.

In between, we saw few irregularities in the results when we increase the number of GPU nodes. At present we are debugging this error. Once we fix it I will create new pull requests and send it to you for approval. So, we will wait on this PR for some time until we fix our code.

[mgduda]

@Pranay-Reddy-Kommera Are the changes in this PR superseded by those in PR #17 ? If so, please feel free to close this PR.

[Pranay-Reddy-Kommera]

@mgduda I am closing this PR.