Skip to content

CCPP Framework Meeting Minutes 2024 04 01

Jump to bottom
Courtney Peverley edited this page Apr 1, 2024 · 6 revisions

Agenda

Attendees: Mike Kavulich,

Code management topics

Clarifying PR merge rules

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

Tagging in CCPP framework development workflow

  • [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?

GitHub issues/PRs

CCPP Framework (issues, PRs, discussions)

Standard names (issues, PRs, discussions)

New items for discussion

Updates from last time

Previous notes

Other business

Meeting notes

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
    • 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

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?
  • Jesse - should notify them somehow (Dom - issue in the corresponding repo) since this could lead to complicated issues that are hard to debug

CCPP Framework wiki

Clone this wiki locally