Skip to content
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

update SCM-only time-vary schemes to use timestep_init phase #562

Merged
merged 3 commits into from
Mar 3, 2021

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented Jan 27, 2021

  • Update SCM-only time-vary schemes to use timestep_init phase
  • Move diagnostic clearing back to SCM code since it has been removed in the timestep_init phase of GFS_phys_time_vary

@grantfirl
Copy link
Collaborator Author

Associated PR: NCAR/ccpp-scm#223

@grantfirl
Copy link
Collaborator Author

The change (reversion) in the CMakeLists.txt file is not meant to stay. It was needed in order to compile with the SCM. With the newer version, cmake can't find the ccpp_prebuild-generated cmake files. This is very likely the result of other errors within the SCM build system, but I couldn't figure it out yet.

That being said, the timestep_init changes compile/run just fine and are all SCM-only. In fact, it seems like there will no longer be a need to have host-specific versions of GFS_rad_time_vary.

@grantfirl grantfirl marked this pull request as ready for review March 1, 2021 22:47
@grantfirl
Copy link
Collaborator Author

The CMakeLists.txt reversion is no longer needed after fixing the SCM build system.

use aerinterp, only : read_aerdata, setindxaer, aerinterpol

use iccn_def, only : ciplin, ccnin, ci_pres
use iccninterp, only : read_cidata, setindxci, ciinterpol

#if 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

This has been implemented for FV3 already, will that follow in another PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. This is only to catch up to ccpp-physics as of late January. More PRs will follow to catch up with FV3 develop and ccpp/physics master.

enddo
enddo

#if 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I know this has been moved/implemented. Will catch up in followup PR.


! Not needed for SCM:
!> - Call gcycle() to repopulate specific time-varying surface properties for AMIP/forecast runs
!if (nscyc > 0) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

For future "reunification of the .fv3. and .scm. versions, are you planning to harcode nscyc/fhcyc to < 0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hadn't thought about that yet, but that may work (like I had hard-coded radiation timesteps to turn it off when needed). There may be an additional issue with gcycle, IIRC. Right now, it's not even being compiled with SCM. I know that the rad_time_vary is now more-or-less identical between fv3 and scm versions, but I'm not sure about the other time_vary routines. Will need to check on that at some point. That would be a followup PR though (to delete the scm-specific versions when the time comes).

physics/GFS_rad_time_vary.scm.F90 Outdated Show resolved Hide resolved
physics/GFS_time_vary_pre.scm.F90 Show resolved Hide resolved
@grantfirl grantfirl requested a review from climbfuji March 2, 2021 18:44
Copy link
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.

Approved. Please wait with the merge for the next ufs-weather-model PR.

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.

2 participants