-
Notifications
You must be signed in to change notification settings - Fork 42
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
Stochastic physics cleanup #47
Conversation
Update my fork with noaa-psd
update to head
update master
The code addresses the CA MPI reproducibility issue: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Phil! Thanks for this, a lot of work has gone in to this one clearly! Regarding the cleanup of the stochastic physics code, I don't know much about the capabilities of the removed code, so I don't have many comments on that piece. But I assume that you know if anyone outside of PSL would be dependent on any of those features?
Regarding the CA code, in update_ca.F90 with this line:
!Return if not allocated:
if(.not. allocated(board) .and. .not. allocated(lives) .and. .not. allocated(board_g) .and. .not. allocated(lives_g))return
What happens if you only run ca_sgs? Or only ca_global?
A small remark, in the global_ca part of update_ca.F90 , newcell=0 looks to be set twice.
It looks like there is a fv3atm submodule update in the stochastic_physics_wrapper.F90 (and atmos_model.F90?) that goes with this PR? Could be good to post that number in the description.
|
the line |
One thing to note is that all of the code clean-up with the spectral transforms does not change the baseline results for the stochastic physics or land perturbation tests. |
I'm a little curious about the io_layout and need for the io domain specification, does it have to be separate from the ca compute domain? |
That is needed for the new way of reading and writing restarts. |
associated fv3atm PR is NOAA-EMC/fv3atm#396 and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a thorough review, I only skimmed through the code changes
This pull request includes a massive cleanup of the stochastic physics code, addition of a new random number generator for the CA code which allows for the same answer for different PE layouts.
Adds restart capability for ca_global, and unit test programs in subdirectory unit_test, which currently only works on hera.
Also, iseed_ca is now an 8-bit integer, so the seed in the workflow can used the same methodology as iseed_sppt, etc.
This also fixes
#33
and
#25