Skip to content

add finalize subroutine for stochastic physics#32

Merged
pjpegion merged 2 commits into
NOAA-PSL:masterfrom
pjpegion:stoch_fix
Nov 28, 2020
Merged

add finalize subroutine for stochastic physics#32
pjpegion merged 2 commits into
NOAA-PSL:masterfrom
pjpegion:stoch_fix

Conversation

@pjpegion
Copy link
Copy Markdown
Collaborator

This a new feature to add a finalize subroutine to deallocate array at the end of a run. Associated with PRs
NOAA-EMC/ufsatm#199 and
ufs-community/ufs-weather-model#278

Copy link
Copy Markdown
Collaborator

@lisa-bengtsson lisa-bengtsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@HenryRWinterbottom
Copy link
Copy Markdown

I would strongly suggest adding if-def type statements to this PR. For example, instead of:

deallocate(variable)

do:

if(allocated(variable)) deallocate(variable)

This better protects the code from possible and difficult to chase segmentation faults in the future -- at least based on my experience.

@pjpegion
Copy link
Copy Markdown
Collaborator Author

@HenryWinterbottom-NOAA good point, I thought about doing it, but was just lazy.

@HenryRWinterbottom
Copy link
Copy Markdown

@pjpegion If you are planning to do it, just send me a note (or submit a new PR) and I am happy to approve.

@pjpegion
Copy link
Copy Markdown
Collaborator Author

@HenryWinterbottom-NOAA I added the if (allocated()) conditionals

Copy link
Copy Markdown

@HenryRWinterbottom HenryRWinterbottom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @pjpegion.

Approved.

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks okay; there are several places where a comment line

! Copy blocked data into contiguous arrays; no need to copy weights in (intent(out))

was moved around to places where it doesn't belong. I recommend removing or changing them, but certainly not a show stopper.

I am currently creating new baselines on cheyenne.intel, cheyenne.gnu and gaea.intel. The new baseline date tag will be 20201127, please update RTPWD in rt.sh accordingly.

@pjpegion
Copy link
Copy Markdown
Collaborator Author

Looks like the changes you are suggesting are for the fv3atm PR.

@climbfuji
Copy link
Copy Markdown
Collaborator

Looks like the changes you are suggesting are for the fv3atm PR.

Yes, sorry.

@pjpegion pjpegion merged commit e4913c0 into NOAA-PSL:master Nov 28, 2020
@pjpegion pjpegion deleted the stoch_fix branch July 30, 2021 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants