-
Notifications
You must be signed in to change notification settings - Fork 67
CoMorph convection scheme refactoring #292
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
Open
MichaelWhitall
wants to merge
9
commits into
MetOffice:main
Choose a base branch
from
MichaelWhitall:comorph_refact
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
352a57d
Copied in all source from the fcm branch
MichaelWhitall 9358789
Merging in changes from git_migration tag
MichaelWhitall c4e4607
Merge branch 'vn3.0_comorph_refact' of github.com:MichaelWhitall/lfri…
MichaelWhitall 564b6b7
Added myself to the contributors.md file.
MichaelWhitall b1491d7
Sync with the vn2.2 branch (changed spelling of "haloes" to "halos").
MichaelWhitall abbf801
Sync with the vn2.2 branch (changed spelling of "haloes" to "halos").
MichaelWhitall eb7b81b
Sync with the vn2.2 branch.
MichaelWhitall 2529025
Sync with the vn2.2 branch.
MichaelWhitall c1e7b1b
Sync with the vn2.2 branch.
MichaelWhitall File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
169 changes: 103 additions & 66 deletions
169
interfaces/physics_schemes_interface/source/kernel/conv_comorph_kernel_mod.F90
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'm not entirely sure I follow this - can you explain?
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 thanks Ian, that was quick! (maybe I was meant to open this in "draft mode" so I could finish documenting it before reviews begin!) I'll try and scribble some more explanations:
cca_3d,ccw_3d) which have not been pre-populated with the input values of the prognostics (ascca_3d0,ccw_3d0are). Subroutinecomorph_conv_cloud_extrasthen setscca_3d0,ccw_3d0to the max out of their existing input values and thecca_3d,ccw_3doutput by CoMorph. It does this on all levels, so the local arrayscca_3d,ccw_3dneed to be set on all levels, but CoMorph doesn't set anything on the top level.fast_physics_alg) from my branch will make conv cloud persist forever, changing answers, so needs to be in my branch.The point of the rearrangement (making CoMorph set local copies instead of setting the prognostics directly) is to make CoMorph agnostic to whether another scheme has already added contributions to the cca, ccw prognostics before it. CoMorph should supplement them not overwrite them.
Cheers!
Mike
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.
Thanks - that makes sense. In which case, I wonder if it's worth wrapping this code inside an "if-comorph" test, so that it's not being done for the 6A scheme where it isn't really needed?
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.
Yep no problem, I've put the cca, ccw initialisation in fast_physics_alg inside an if test on comorph as suggested: fast_physics_alg_mod.X90