-
Notifications
You must be signed in to change notification settings - Fork 82
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
Remove memory spike in Sampling #1235
Conversation
This is great except that it now all ranks need to know about the probe locations. So it ends up needing more memory in total though there is no spike. The next step that I want to do before this PR is merged is to have the ranks only need to know about probe locations that are relevant to it. Basically I want to pass |
ac8f6e4
to
fc43780
Compare
Status update before I leave this aside for a bit:
|
fc43780
to
1f1a69a
Compare
5f5c8ff
to
fd05e86
Compare
5a54ba1
to
c99cba2
Compare
Progress is being made... I need to fix the m_fill_val in radar sampler to be the min of the domain (checking to see if that's allowed). And then netcdf builds need to be fixed with the realvect change. And then finally I can call |
0ce7375
to
d781798
Compare
c989db1
to
7a8436e
Compare
We are close. I added the box check which is the thing that should help the memory pressure. I need to check it on a large case and see if it actually works/causes no diffs. The only other thing I am "worried" about is the radar sampler since I don't have an input file for that. |
1655d25
to
171b62a
Compare
Ran the new reg test with MPI and everything on and I don't see any diffs in the particle files. I would like to double check netcdf. But here's the upshot:
This PR
|
This is the diff with the big case I am using to benchmark (40M probes):
|
Checklist:
|
not sure if you've incorporated my additions to probe sampler, but hopefully those won't be too bad to address. |
Yup I merged those in when you made the changes. You had good tests in there that made it easy. |
That last one made the init 10% faster than the one before.
Not enough to claim that the 80s were right about greed. I will keep it though. |
Free surface sampler looks fine. |
abl_sampling_netcdf with 10 ranks on Kestrel:
Things "look" right:
But I would like to have an |
a13b9c5
to
df9f910
Compare
got nccmp added to kestrel:
good to go for netcdf |
df9f910
to
da7a9f7
Compare
I checked one of our cases which crashed before on CPU in the first step. I changed the output format to native and it has already run fine for 100 seconds. Will keep you updated. |
ebf5ccd
to
0d55040
Compare
ec868b1
to
c51a440
Compare
fdf495c
to
a2d9ef3
Compare
With some final tweaks, I think we are good to go. Running the GPU tests now. |
Summary
This removes the memory spike on the ioproc when doing sampling by initializing the sampling particles in parallel.
Pull request type
Please check the type of change introduced:
Checklist
The following is included:
This PR was tested by running:
Additional background
Issue Number: #1186