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

Ca develop #418

Merged
merged 11 commits into from
May 4, 2020
Merged

Ca develop #418

merged 11 commits into from
May 4, 2020

Conversation

lisa-bengtsson
Copy link
Collaborator

I have made updates to the CA scheme for samfdeepcnv convection, and removed the isppt logic (this will be added in a later update in a more consistent manner).

@climbfuji
Copy link
Collaborator

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.

@grantfirl @llpcarson for your information

!endif
endif

if (do_ca .and. ca_global) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems to me that lines 165 through 176 are not dependent on the time step information, i.e. they don't change during the model run. Is this correct? If so, it would be better to create or populate the GFS_stochastic_init subroutine in the same file that does these computations only once.

I see that in your fv3atm PR, you are doing the corresponding computations in GFS_initialize in lines 474-489 (which is also only done once at the beginning).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi Dom,

I could not get moving the code to the GFS_stochastic_init routine to reproduce the results, maybe I have to move vfact_ca to "Init_param" instead of "Coupling"?

Nevertheless, I got it to work (reproduce results) by making vfact_ca an input variable in GFS_stochastic_run by defining it in GFS_typedefs.meta and passing in kdt and vfact_ca and only doing this computation if(kdt ==1)then. Let me know if you think this is a satisfactory solution? I can work with you to get this code into the GFS_stochastic_init when we have a bit more time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pinging @grantfirl and @llpcarson as Dom is on leave until May 10th.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lisa-bengtsson this is totally fine, I might have sent you down the wrong way. You didn't push those changes yet, please do so.

Copy link
Collaborator

@JongilHan66 JongilHan66 left a comment

Choose a reason for hiding this comment

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

Changes on samfdeepcnv.f look ok.

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.

This looks ok to me, but see my comment here NOAA-EMC/fv3atm#87 (comment) on how you could implement my original suggestion (and save memory).

@llpcarson llpcarson merged commit 25a72ec into NCAR:master May 4, 2020
character(len=*), intent(out) :: errmsg
integer, intent(out) :: errflg

!--- local variables
integer :: k, i
real(kind=kind_phys) :: upert, vpert, tpert, qpert, qnew, sppt_vwt
real(kind=kind_phys), dimension(1:im,1:km) :: ca
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lisa-bengtsson I know this has been merged already, but in case ca is not always allocated when running GFS_stochastics.F90, it would be good to change this to

real(kind=kind_phys), dimension(:,:) :: ca

Did the comment I make in FV3 regarding moving the vfact_ca to the GFS_control_type (i.e. to Model%vfact_ca) make sense to you?

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