Skip to content

Conversation

@weiyuan-jiang
Copy link
Contributor

@weiyuan-jiang weiyuan-jiang commented Aug 15, 2025

Types of change(s)

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Trivial change (affects only documentation or cleanup)
  • Refactor (no functional changes, no api changes)

Checklist

  • Tested this change with a run of GEOSgcm
  • Ran the Unit Tests (make tests)

Description

Needed for GEOS-ESM/GEOSgcm_GridComp#1143

Related Issue

@weiyuan-jiang weiyuan-jiang requested a review from a team as a code owner August 15, 2025 13:47
@weiyuan-jiang weiyuan-jiang added the 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. label Aug 15, 2025
@tclune
Copy link
Collaborator

tclune commented Aug 15, 2025

I want @atrayano to look at this. The change is very specific to Land and LocStream is meant to be somewhat generic.

@gmao-rreichle gmao-rreichle added the 🎁 New Feature This is a new feature label Dec 3, 2025
@gmao-rreichle
Copy link
Contributor

I want @atrayano to look at this. The change is very specific to Land and LocStream is meant to be somewhat generic.

@tclune, @atrayano : The change is needed for the addition of river routing (GEOS-ESM/GEOSgcm_GridComp#1143) to the model. For now, we're only targeting land tiles for routing (because we've always had the Pfafstetter network for land), but the idea is to also include runoff routing from landice tiles, now that GEOSldas can include landice tiles. Lauren is working on creating Pfafstetter indices for landice tiles. So the change isn't specific to land per se, although it is specific to Surface. Admittedly, ocean tiles don't have a Pfafstetter index (pfaf_index). I'm unsure how we'll handle lake tiles when we get to crossing that bridge. Currently, there are "fake" catchments within large lakes such as the Great Lakes. These "fake" catchments assist with routing water through the lakes.

@tclune
Copy link
Collaborator

tclune commented Dec 3, 2025

If I have understood correctly, the "ESMF way" that this would be handled would be for the Pfafstetter indices to be another ESMF_Field on the same ESMF_LocStream. (I.e., the loc stream remains general, and it is just another field that is created and managed associated with that locstream.)

I am less familiar with how this analog is done with MAPL_LocStream, but am guessing that there is no analog to a "Field" here, and instead MAPL_LocStream is a fusion between geometry and data, and more hardwired in practice. I'll try to discuss with @atrayano asap so that the question does not linger.

Since MAPL3 has a better way to deal with this, I probably am not opposed to this change. But if there is a better way even in MAPL2 I'd like to push for that.

tclune
tclune previously approved these changes Dec 3, 2025
@gmao-rreichle
Copy link
Contributor

Adding Atanas' concurrence for the record, just in case. For some reason, I don't see it showing up on the website as of 4:41pm:
Atanas

@mathomp4
Copy link
Member

mathomp4 commented Dec 4, 2025

@weiyuan-jiang @gmao-rreichle Would you like a release of MAPL with this?

Also, do you need #3973 in this as well? I see it also involves EASE grid like this PR seems to.

@gmao-rreichle
Copy link
Contributor

@mathomp4 : Yes, we would need a new release. Sorry for the extra work. I defer to @weiyuan-jiang and @biljanaorescanin re. #3973

@weiyuan-jiang
Copy link
Contributor Author

That PR is not necessary.

@mathomp4 mathomp4 merged commit a761bfa into develop Dec 5, 2025
40 of 41 checks passed
@mathomp4 mathomp4 deleted the feature/wjiang/pfaf_index branch December 5, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0 Diff The changes in this pull request have verified to be zero-diff with the target branch. 🎁 New Feature This is a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants