-
Notifications
You must be signed in to change notification settings - Fork 64
CCPP Framework Meeting Minutes 2024 04 01
Courtney Peverley edited this page Apr 1, 2024
·
6 revisions
Attendees: Mike Kavulich,
Two PRs were merged (by me) without being fully tested in the UFS weather model
This caused some consternation.
We need a written procedure for when PRs should be merged
- Current Wiki instructions are horribly out-of-date, should be updated
- Ideally, there should be some sort of real indication on a framework PR for when it is actually ready to be merged.
- Label?
- Manual comments?
- Can it be automated somehow?
- Or, as suggested by Courtney (https://github.com/NCAR/ccpp-framework/issues/553), a develop branch where things go after framework tests pass and PR is approved, and then a main branch subject to a different development process
- [Courtney] (related to https://github.com/NCAR/ccpp-framework/issues/553) - how do we approach tagging in CCPP-framework? Some thoughts from meeting with Dustin/Dom/Steve/Courtney:
- NCAR proposal was to tag all PR commits - too often?
- only tag when we need them? with strict naming conventions so we can "filter" them out easily from tag list
- only main branch tagged? develop branch uses hashes?
- do tags need to (sometimes?) specify the host it's for?
- when does review happen? develop? and then also main? just main - or would that review be too long?
CCPP Framework (issues, PRs, discussions)
-
PR 523: Add support to use mpi_f08 MPI module
- Merged, MPI now required for code using latest framework
-
Issue 543: Remove run phase requirement from capgen
- PR now open: Add support for scheme w/o run phase
- Merged
- Issue 540: ccpp_prebuild: (re-)introduce optional attribute
-
Issue 547: Post capgen unification: pylint code
- Still a good idea
-
PR 549: Constituent updates
- Needs reviews
-
Issue 553: Constituent updates
- Discussed above
Standard names (issues, PRs, discussions)
-
Issue 26: Units for relative humidity
- Will open PR discussed last week soon
-
Issue 57: Feedback on the meaning of: sea_ice_area_fraction_of_sea_area_fraction
- Needs some more attention; this has some complicated subtleties
- How do we balance ideal standard names with their real-world use?
-
Issue 58: Variable name changing policy and procedures
- Who should we reach out to for adding code owners?
- Issue 59: New standard names for land surface variables
- Issue 61: Clarification regarding _at_interface suffix
-
PR 62: Update Standard Name Rules table for dimensionless units, change variables improperly using "1" to "frac"
- Should we mandate change from "frac" to "fraction"
- Issue 63: Dealing with unusual units
- Issue 64: Need to replace the use of "volumetric" with "volume_fraction" when referring to soil quantities
Michael K: Merged PRs without being fully tested - need better way to track PR status
- Wiki instructions out of date (very) - how should we update?
- Grant & Ligia - do we follow the same procedure for other UFS repos?
- Grant - no huge issue in having framework advance at its pace and then periodically update the hash & issue necessary bugfix(es)
Courtney P: develop branch in addition to main branch
- Seems to be agreement on the usefulness of this
- Some question on importance of tagging
- Cheryl - CESM uses tags to identify all externals; we find it the best way to keep track of all the different versions of CAM
- Easy to tell on sight where the tag they’re using sits in our progression (is it before the one I’m using?) vs hash doesn’t really tell you that info
- Michael K - what is the cadence of tags? Prefers to minimize tags
- Seems redundant to tag everything when there’s already a hash
- Don’t see a problem with tags in general, just don’t want hundreds of them cluttering everything up
- Cheryl - propose that when we (CESM) want a tag, we tag the hash we want (w/ semantic versioning)
- Would the semantic versioning be tied to some level of testing?
- Jesse - tagging everything is more human-readable, easier to debug older versions
- Also want to ensure both UFS and NCAR are using the exact same release so we don’t diverge (as much as possible!)
- Dom - when you tag everything, you get many pages on tags page and it’s no longer really usable
- Want to have semantic v.x.y.z versioning as core release tags. Anything else could have a different tag but wouldn’t follow that format
- How do we handle tags like ufs-srw-v2.0.0?
- Michael K - when srw app release 2.0, we didn’t want to point to a bare hash and didn’t want to create a release branch
- Jesse - back to testing discussion
- Every host model that cares about it should be testing at stage where we’re merging develop -> main
- Should try our best for every host model to have tested with every PR to be merged into main
- Michael K - ideally, framework drives development. In reality, things are more inward
- Possibility of a circular dependency?
- Every PR to main now has a lot of unrelated changes - what will that mean for testing?
- Will be possible pain point to be testing something that is not your development and trying to debug
- Michael K - ideally, framework drives development. In reality, things are more inward
- Ligia - PR into main merged, host will grab the tag/hash?
- Grant - yes. UFS will be updated to grab the new hash
- Dom - if you have changes in the framework that go with changes in the physics, then you’ll have to wait for the develop -> main merge?
- Jesse - could potentially point to develop tag (+test it separately)
- Cheryl - in CAM, we allow developer to move forward and test with development/temporary tag until the moment the PR is made (point to develop tag during development all the way up until PR is made)
- Michael K - how do we get around asynchronous way development happens?
- Michael W - won’t get to perfect solution (esp. with shared community framework)
- We can try minimally-invasive solution and re-iterate if we decide we don’t like it
- Michael K - PR problem with accelerated development - people don’t like sudden changes!
- Dom - could have main, dev_sima, dev_ufs
- Only works if you have a committed code manager for each dev branch
- Also potentially could lead to divergence
- Matt - have we considered doing periodical, scheduled releases?
- Dom - thought about it, but didn’t want to force waiting for release
- Moral of the story: we need to figure out the cadence of tagging and/or what we tag and what we don’t
- 3 options:
- Keep what we have (possibly add tagging?)
- NCAR proposal - one, shared develop branch
- Dom’s suggestion of 2 dev branches
- 3 options:
- Cheryl - CESM uses tags to identify all externals; we find it the best way to keep track of all the different versions of CAM
Michael K: Discussion about sea_ice_area_fraction_of_sea_area_fraction
- Standard names for area are ambiguous
- ccpp-physics uses standard names in a way that’s inconsistent with how they’re supposed to be used
- Hoping other people will weigh in; will open new issue
- How high is the priority of this? How do we enforce proper use of standard names?
- Dom - have to force them / tell them to correct it
- This seems like a burden for us; how can we nip these in the bud?
- Dom - have to force them / tell them to correct it
- Jesse - should notify them somehow (Dom - issue in the corresponding repo) since this could lead to complicated issues that are hard to debug