Reduce the memory usage that is important for ne1024 simulation

[sjsprecious]

This PR reduces CMEPS mediator memory usage, which is critical for ultra-high-resolution simulations such as ne1024. All code changes were made by Claude under my supervision.

In configurations where lnd→rof is the only grid-crossing conservative coupling map, computing the mapping weights online during mediator initialization (ESMF_FieldRegridStore → GeomRendezvous → Zoltan_RCB) produces a per-rank memory spike that OOM-kills the job before the first time step. This PR allows the lnd→rof conservative map (the consd fraction map generated by the --norm_type dstarea flag) to be read from a single pre-computed offline mapping file via the existing LND2ROF_FMAPNAME XML variable, and additionally eliminates a duplicated ESMF mesh in the atmosphere/ocean flux computation.

Code Changes

1. Hook up LND2ROF_FMAPNAME for the lnd→rof flux mappings — bug fix.

The lnd2rof_map attribute (driven by the pre-existing LND2ROF_FMAPNAME XML variable) was read but never used: the five addmap_from calls for the runoff/irrigation fields (Flrl_rofsur, Flrl_rofi, Flrl_rofgwl, Flrl_rofsub, Flrl_irrig) hardcoded unset, so the mapping was always computed online. These calls now pass lnd2rof_map.

2. Read the lnd→rof fraction map from the same file — new capability.

med_fraction_init builds a separate consd lnd→rof route handle online. It now queries the same lnd2rof_map attribute: if it names a mapping file, the weights are read from that file (aborting with a clear error if the file does not exist); if unset or idmap, the original online path is used unchanged. As discussed in #408 , consd and consf mappings give the same results within roundoff in CMEPS due to the normalization applied when the weights are used, so a single mapping file serves both. The XML/namelist documentation recommends generating this file with --norm_type dstarea, since land meshes generally do not carry a mask.

3. Plumbing.

med_map_routehandles_initfrom_fieldbundle gains an optional mapfile argument, forwarded to the field-level routine that already supported reading offline weights via ESMF_FieldSMMStore. No behavior change when the argument is absent.

4. Avoid a duplicated aoflux mesh.

In the ogrid path, aoflux_mesh now reuses the ocean field's existing mesh handle instead of building a full duplicate mesh per rank with ESMF_MeshCreate — a meaningful memory savings at high resolution. This is safe because the source mesh persists in FBArea(compocn) and aoflux_mesh is never destroyed.

5. Documentation updates for LND2ROF_FMAPNAME and lnd2rof_map describing the dual consd/consf use and the recommended --norm_type dstarea generation flag.

Answer changes

1. The answers are bit-for-bit between this PR and the main branch when using online mappings (the default).

2. There are roundoff-level differences when comparing runs with online mappings (the default) against runs where LND2ROF_FMAPNAME points to an offline-generated mapping file. (Even though you could, in principle, get bit-for-bit results with online vs. offline mapping files, the current implementation uses a single lnd2rof mapping file for both the consf and consd mappings in CMEPS. These two mapping types are roundoff-level different from each other (but not more than that because the lnd mesh typically doesn't have a mask), leading to roundoff-level differences due to using a single file for these two mapping types.)

[billsacks]

@mvertens - If you have a chance, I'd like to hear any thoughts you have on this change. (I have just skimmed it so far - haven't had a chance to look closely yet.)

[billsacks]

The overall changes here look good to me. I appreciate your plugging in the use of lnd2rof_map, which seems like it previously wasn't hooked up. And I think I see why you needed to introduce a separate lnd2rof_fmap (though see my comments asking for this to be made more explicit in some documentation).

I do have a few requests... many about editing some comments, but a couple slightly more substantial... but still overall minor - for the most part this looks good to me.

Once you make the final changes, I'd like to see that at least one or two tests have been run with baseline comparisons to verify that these changes work and are bit-for-bit in out-of-the-box configurations. One test that I think would cover all of these changes would be a B compset test with the aoflux grid set to ogrid (the setting of ogrid is needed to cover the changes in med_phases_aofluxes_mod, and should still cover the other changes here). I'd like to see that run with comparisons against a baseline.

[jtruesdal]

@sjsprecious and @billsacks I was also thinking about a test to show that the offline and inline are identical but in the past I've run into roundoff errors in the way the model calculates the mesh compared to offline. If that's the case here you may want to implement some type of tolerance. Maybe a nonissue if they are identical in this case. Other than that I think with the current set of updates the code looks good to me.

[sjsprecious]

Thanks @jtruesdal for your comments. I think one thing that can contribute to the difference here is that users may use different compiler/ESMF versions when generating the offline mesh.

Once Bill provides the instructions about running the tests he suggested, I will let you know at least whether the code changes affect the current behavior.

[billsacks]

The latest set of changes looks good - thank you @sjsprecious for making those changes.

A piece I am less confident about is whether the assignment of aoflux_mesh to lmesh (as opposed to doing an ESMF_MeshCreate there) is always safe. I am asking the ESMF group about that, along with the similar change you made in CDEPS.

Regarding testing, a good test would be SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid, since this covers the change to med_phases_aofluxes_mod in addition to your other changes. This is in the prebeta test list. It hasn't been run on recent versions of the code, so you'll need to generate baselines and then run with your latest changes with comparisons against baselines.

It would also be good to run aux_cime_baselines, or at least keep a careful eye on aux_cime_baselines when they are run in their nightly run following the merge of this PR. As with the associated CDEPS PR, @fischer-ncar could provide some input here on testing.

Regarding @jtruesdal 's point: I wasn't thinking of something as rigorous as testing behavior with online vs. offline generation of mappings - I'm mainly wanting to confirm that the changes here don't break anything or change answers for the case where we're still using online mapping.

[billsacks]

I talked with @mvertens today.

The good news is that she feels that this is NOT a problem:

A piece I am less confident about is whether the assignment of aoflux_mesh to lmesh (as opposed to doing an ESMF_MeshCreate there) is always safe. I am asking the ESMF group about that, along with the similar change you made in CDEPS.

However, @mvertens raised a question / concern about having two different mapping files: LND2ROF_FRAC_FMAPNAME and LND2ROF_FMAPNAME. I think I see why this was done, but @mvertens questioned whether this was necessary.

Our initial question on this is: What is the difference in how these mapping files are generated? My sense is that LND2ROF_FMAPNAME should be generated with --norm_type fracarea whereas LND2ROF_FRAC_FMAPNAME should be generated with --norm_type dstarea (or no --norm_type, since dstarea is the default). Is that what was done? If so, can you add some documentation of this difference to the relevant xml and namelist definition documentation? We are also wondering if the resulting mapping files actually differ based on using these different --norm_type flags. Can you comment on this?

This discussion led me to find these related issues:

• Change mapconsf mappings to mapconsd? #408
• Have temporary ability to set lfrac mapping for mizuRoute from mapconsd to mapconsf #409
• CTSM runoff remapping issue again mizuRoute#426

The summary of those issues is: Even though we currently have different mapping types for different lnd2rof mappings - and that's what you're supporting in the changes in this PR - this is considered to be a bug rather than a feature, and in fact we want to be using the same mapping type for all the mappings. So now I'm not sure what the right path forward is. I can see a few paths forward, which I lay out below... this probably needs some more discussion to figure out how we want to proceed.

(1) Keep the current changes and merge this as is

Pros:

• Allows high-res runs to be done from main branch
• Specifying pre-made mapping files will give the same behavior as you currently get

Cons:

• Confusing for people using this, since the differences between the two LND2ROF maps are subtle, and extra work is needed to create both maps - and a user needs to know that they need to create both maps)
• This will probably be reworked in the future, so we'll need to rework the changes here at that point

(2) Make an initial change where we change all of the lnd2rof mapping to be consd instead of consf, then change this PR to use a single LND2ROF mapping file and merge it

Pros:

• Allows high-res runs to be done from main branch
• Specifying pre-made mapping files will give the same behavior as online mapping (though note that this behavior will differ from the current behavior)
• Simplifies the LND2ROF mapping files
• Moves us in the direction we want to go, according to Change mapconsf mappings to mapconsd? #408

Cons:

• I'd like to have more discussion of whether it's really correct to change all of the consf mappings to be consd as suggested in Change mapconsf mappings to mapconsd? #408... so this will take more time
• The initial step may be answer changing; would want to investigate by how much
• If anyone has lnd2rof mapping files already, and if these were generated with --norm_type fracarea, they will need to be regenerated with --norm_type dstarea

(3) Don't change online mapping behavior for now, but use the same mapping file for the lnd2rof frac mapping as for the other lnd2rof mappings... this relies on the assumption that the differences between consf and consd mapping are inconsequential (which #408 suggests, but I'm not sure it applies in all cases). Bring this to main in this way.

Pros:

• Allows high-res runs to be done from main branch
• Simplifies the LND2ROF mapping files
• Moves us forward in the right direction, though not as far as option (2)

Cons:

• Answers could differ between using a pre-computed mapping file vs. online mapping; this may be doing (slightly) the wrong thing for some of the lnd2rof mappings when using a pre-computed mapping file. We'd want to verify that the differences are small.
• Puts some pressure on to resolve Change mapconsf mappings to mapconsd? #408 (at least for the lnd2rof mappings) to remove the above con.

(4) Defer bringing in these changes until post CESM3

Pros:

• Avoids both (a) changing CESM results for now and (b) making changes that might be confusing or need to be redone later.
• Specifying pre-made mapping files will give the same behavior as you currently get

Cons:

• High-res runs will need to be done using this branch, which may need to be maintained for some period

(5) Hybrid between (1) and (4): bring in the changes on this branch that do NOT rely on the new mapping file (LND2ROF_FRAC_FMAPNAME); leave those additional changes on a branch for now.

[sjsprecious]

Thanks @billsacks .

Our initial question on this is: What is the difference in how these mapping files are generated? My sense is that LND2ROF_FMAPNAME should be generated with --norm_type fracarea whereas LND2ROF_FRAC_FMAPNAME should be generated with --norm_type dstarea (or no --norm_type, since dstarea is the default). Is that what was done? If so, can you add some documentation of this difference to the relevant xml and namelist definition documentation?

Yes, LND2ROF_FMAPNAME is generated with --norm_type fracarea whereas LND2ROF_FRAC_FMAPNAME is generated with --norm_type dstarea. I have updated the comments in the XML files as suggested.

We are also wondering if the resulting mapping files actually differ based on using these different --norm_type flags. Can you comment on this?

Most fields between these two mapping files using different --norm_type options are identical. But the weights variable S does differ around 1.45e-13 (maximum absolute difference) or 6.7e-13 (maximum relative difference). Thus I would say they are not exactly the same but the difference seems subtle or at roundoff level.

So now I'm not sure what the right path forward is. I can see a few paths forward, which I lay out below... this probably needs some more discussion to figure out how we want to proceed.

I appreciate that you list all the available options here and provide detailed explanations. My two cents is that if my changes pass the regression tests that you suggested, it means that they do not change the current CMEPS behavior and should be safe to bring into CESM. I understand that providing two similar mapping files may be confusing, but if those XML variables are unset, the online mapping is used and the user may not even notice the existence of those XML variables.

Regarding the option to use the same consd or lnd2rof mapping, I feel that it is out of the scope of this PR and should be addressed by a separate PR (you have also listed a few related open issues), especially that those revisions may cause answer changing while this PR is expected to be BFB. I think it is fine to revise some work from PR afterwards once you decide how to address those issues.

[sjsprecious]

Regarding testing, a good test would be SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid, since this covers the change to med_phases_aofluxes_mod in addition to your other changes. This is in the prebeta test list. It hasn't been run on recent versions of the code, so you'll need to generate baselines and then run with your latest changes with comparisons against baselines.

Thanks @billsacks . So I did the following steps:

1. Checked out the CESM cesm3.0-alphabranch branch (commit: fafcb75) with the latest cmeps1.1.51 tag.
2. Generated a new baseline for the test SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid on Derecho at /glade/derecho/scratch/sunjian/cesm_baselines/cesm_baseline_pr414.
3. Checked out the same CESM cesm3.0-alphabranch branch (commit: fafcb75) but with my CMEPS changes here.
4. Generated a test run for SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid on Derecho at /glade/derecho/scratch/sunjian/SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid.C.20260630_224152_f094m9 and compared to the baseline in Step 2.
5. The comparison showed that except a namelist difference, all the results were identical.

  SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid (Overall: NLFAIL) details:
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid CREATE_NEWCASE
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid XML
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid SETUP
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid SHAREDLIB_BUILD time=233
    FAIL SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid NLCOMP
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid MODEL_BUILD time=259
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid SUBMIT
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid RUN time=790
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid BASELINE cesm_baseline_pr414:
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid TPUTCOMP
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid MEMLEAK insufficient data for memleak test
    PASS SMS_D_Ld1.ne30pg3_t232.B1850C_LTso.derecho_intel.allactive-aoflux_ogrid SHORT_TERM_ARCHIVER

[billsacks]

@sjsprecious - sorry that we've had a lot of back and forths here, but I think @mvertens and I have more clarity on the path forward based on our discussions and investigations this week.

First, thanks for the responses and the test you have run.

We realized that, with the way we think you're generating the offline mapping file, the two files you're providing here will always be effectively the same, within roundoff. That's because we think the land mesh you're using probably doesn't have a mask: that's the case for land meshes in general in CESM: the land mask is added at runtime based on the ocean grid/mask. When you have an unmasked source mesh, fracarea and dstarea mapping give identical results (within roundoff), so there is no value in providing these two separately. Furthermore, the fracarea-based map that you generate offline will differ from the fracarea-based map that's generated in the typical online workflow.

This made us worry at first that you would get wrong answers with this offline fracarea-based map, but after digging more deeply, we have convinced ourselves that this is probably okay - for the reason that #408 asserts that consf mappings can safely change to consd. (Briefly: there is a normalization applied in the course of applying the mapping weights that makes it irrelevant whether the underlying mappings were created using dstarea or fracarea normalization.)

What this all means for this PR is:

(1) We feel that it is confusing to have these two separate lnd2rof mapping files, since (a) they will always be identical given that the land mesh doesn't have a mask, (b) the fracarea one isn't really doing what fracarea is designed to do (given the lack of a land mask), and (c) it doesn't actually matter (beyond roundoff) whether you're using fracarea or dstarea normalization in these contexts. So we'd like you to remove the new xml and namelist variable, just using the single existing lnd2rof mapping file variable in all places in the code.

(2) We should change the description of this single lnd2rof variable to explain this. I can help with this, but before spending time on this, let's see what (3) shows.

(3) Since we aren't totally confident here, we feel it's important to see a demonstration that the lnd2rof mapped variables are the same - within roundoff - when providing an offline mapping file as they are when doing the standard online mapping (like @jtruesdal suggested). I'm thinking this could be easier if (a) you use a coarse grid so you don't run into the problems you have run into at this high resolution... just use the same method for creating the mapping file, but applied to the coarse grid; (b) if you use an I compset for a cheaper run and to avoid feedbacks, so I think changes in the LND2ROF fields shouldn't feed back to the rest of the system (an F compset would also avoid these feedbacks but would be much more expensive); and (c) turn on cplhist files (HIST_OPTION / HIST_N) so you can look at the mapped lnd2rof fields on the rof grid (looking at these fields for just one time step a few hours or more into a run will be sufficient). Seeing that these mapped lnd2rof fields are the same within roundoff when you do vs. don't provide a mapping file will give confidence that it's safe to use a single offline-generated mapping file here.

Let me know if you have questions about this.

[billsacks]

(Referring back to my comment from a few days ago, we're suggesting going with option (3). Based on my analysis, the con listed there of doing slightly the wrong thing when using a pre-computed mapping file is actually a non-issue, but I'd like for this to be confirmed with a test run. Until we resolve #408, there is potential for confusion, in that we're using the same offline mapping file for two different mappings in the code, but hopefully a well-written comment can address this confusion, and it should be less confusing than needing to provide two different mapping files that end up being identical.)

[sjsprecious]

Ok, so I did the suggested simulations for option (3):

1. Checked out the CESM cesm3.0-alphabranch branch (commit: fafcb75) with the latest cmeps1.1.51 tag.
2. Generated a baseline for the FHISTC_LTso compset at ne30pg3 resolution on Derecho at /glade/derecho/scratch/sunjian/cam7/run/FHISTC_LTso.ne30pg3_ne30pg3_mg17.derecho.intel.mpich.gpu00_mpi128.online_mapping, using the default online mapping option and simulating one day.
3. Generated a test run for the same FHISTC_LTso compset at ne30pg3 resolution on Derecho at /glade/derecho/scratch/sunjian/cam7/run/FHISTC_LTso.ne30pg3_ne30pg3_mg17.derecho.intel.mpich.gpu00_mpi128.offline_mapping, using the offline LND2ROF and ROF2LND map files and simulating one day.
4. The comparison of cplhist files between these two runs shows that they are identical.

Note that I used the same esmf/8.9.1 version to generate the offline map files or do the online mapping.

[billsacks]

@sjsprecious thank you for doing this testing. However, I think I didn't clearly explain what I was thinking here. For the test run to be meaningful, you'll need to use the CMEPS branch in this PR rather than the main CMEPS branch. Then set both LND2ROF_FMAPNAME and LND2ROF_FRAC_FMAPNAME to point to your lnd2rof mapping file. As noted above, it shouldn't matter which one you use, since the two files you generated only differ by roundoff.

This is important because, as far as I can tell, LND2ROF_FMAPNAME wasn't actually hooked up to anything before your PR.

You can use the same baseline that you already generated. I expect to see roundoff-level changes from the baseline... I'll actually be surprised if this is bit-for-bit. If it's unclear whether answer changes are just roundoff-level, then it could be helpful to do two additional tests - one where you just set LND2ROF_FMAPNAME and the other where you just set LND2ROF_FRAC_FMAPNAME - so we can see which of these leads to possibly-greater-than-roundoff-level changes. But I think it makes sense to start by hoping for the best and setting both of those to point to the same file.

Does that testing make sense to you? Let me know if you have any questions.

[sjsprecious]

Thanks @billsacks for your clarification. Sorry that I misunderstood your points before.

So I did the following new runs with the CESM cesm3.0-alphabranch branch (commit: fafcb75) but with my CMEPS branch in this PR:

• Test 1: the same compset and resolution as the baseline, but setting both LND2ROF_FMAPNAME and LND2ROF_FRAC_FMAPNAME to /glade/campaign/cisl/asap/sunjian/ne30_cdf5_input/map_ne30pg3_TO_r05_consf_c260702.nc. The cplhist output differs from the baseline and 13 of 342 fields have non-zero differences. Those fields are:

Med_frac_rof_lfrac
Med_frac_rof_lfrin
rofExp_Flrl_rofsur/rofsub/rofgwl/rofi/irrig
rofImp_Forr_rofl/rofi
rofImp_Flrr_volr/volrmch
lndExp_Flrr_volr/volrmch

The normalized RMS is between 2.4E-14 and 7.8E-12 and the absolute RMS diffs is between 1E-20 and 1E-17.

• Test 2: the same compset and resolution as the baseline, but only setting LND2ROF_FMAPNAME to /glade/campaign/cisl/asap/sunjian/ne30_cdf5_input/map_ne30pg3_TO_r05_consf_c260702.nc. The cplhist output is BFB identical with from the baseline, which is expected since the only difference here against the baseline is using online or offline mapping file.

• Test 3: the same compset and resolution as the baseline, but only setting LND2ROF_FRAC_FMAPNAME to /glade/campaign/cisl/asap/sunjian/ne30_cdf5_input/map_ne30pg3_TO_r05_consf_c260702.nc. The cplhist output differs from the baseline and the same 13 fields identified in Test 1 have non-zero differences (actually Test 3 is equivalent to the Test 1). This is expected because I am using the consf mapping file for LND2ROF_FRAC_FMAPNAME XML variable instead.

Thank you and let me know if you have any questions.

[billsacks]

@sjsprecious - thanks a lot. This is exactly what I hoped to see. (I also did a quick check myself of your case /glade/derecho/scratch/sunjian/cam7/run/FHISTC_LTso.ne30pg3_ne30pg3_mg17.derecho.intel.mpich.gpu00_mpi128.offline_mapping_v2 and the cpl hist file there, to confirm that it's as expected.)

This gives me good confidence that the plan in my earlier comment is what we want to do here.

So can you please do the following:

(1) Remove the new xml and namelist variable, just using the single existing lnd2rof mapping file variable in all places in the code.

(2) Change the description of this single lnd2rof variable to explain this... something like the following: "lnd2rof flux (conservative) mapping file. (This mapping file is used for both consd and consf mappings; as discussed in #408, the results of these two mappings are the same within roundoff in CMEPS due to the normalization that is done when applying the mapping weights. For simplicity, and because land mesh files generally don't have a mask, this mapping file should be generated with '--norm_type dstarea'." (I would leave out the mention of mizuRoute in the description since this is now being used more generally.)

(3) Redo your test run with the latest code to verify that this change is working as intended: I think that, with the changes I'm suggesting, if you set the single LND2ROF_FMAPNAME to point to the mapping file, results should be bit-for-bit with "Test 1" from your previous comment.

At that point, I think this will be ready to go. Thank you for your continued work on this!

[sjsprecious]

Thanks @billsacks . I have updated my PR with your suggestions (88242a6).

I did another test run with my latest CMEPS branch, setting LND2ROF_FMAPNAME to the /glade/campaign/cisl/asap/sunjian/ne30_cdf5_input/map_ne30pg3_TO_r05_consf_c260702.nc mapping file. The output is available at /glade/derecho/scratch/sunjian/cam7/run/FHISTC_LTso.ne30pg3_ne30pg3_mg17.derecho.intel.mpich.gpu00_mpi128.offline_mapping_v5/run and it is BFB identical with the "Test 1" in my previous comment as you suggested.

Just want to clarify: I chose the consf mapping file for test, but the consd mapping file should be used instead in a real run since the --norm_type dstarea flag is suggested in the XML description.

[billsacks]

Thank you for the change and for doing the additional test. Thanks also for the clarification that you used the consf mapping file for your test. My understanding from one of your earlier comments is that the dstarea and fracarea mapping files are themselves the same within roundoff, so I'm fine with the testing you've done at this point.

I re-reviewed the changes and am happy with them at this point. Thank you for iterating with us on these changes!

[billsacks]

@mvertens - at this point, I feel that these changes are reasonably straightforward and in line with what you and I discussed, so I don't feel that this needs a formal review from you.

[billsacks]

@sjsprecious - one last request: can you please edit the top-level PR description (which I'll also use in the merge commit message) to reflect the latest set of changes? At that point, I'll go ahead and merge this. Thanks again!

[sjsprecious]

Thanks @billsacks . I have updated the PR description as you suggested for the latest changes.

Also do you want me to close the following issues with this PR?

• Change mapconsf mappings to mapconsd? #408
• Have temporary ability to set lfrac mapping for mizuRoute from mapconsd to mapconsf #409
• CTSM runoff remapping issue again mizuRoute#426

Thank you very much for your time and detailed instructions on this work.

[billsacks]

@sjsprecious - thank you for updating the PR description. What you had written under the answer changing section didn't seem quite right. I have edited it. Can you read my edited version of "answer changes" in the top-level PR comment and confirm that this seems correct?

Regarding the issues: thanks for checking. I just closed #409 as wontfix, since it was suggesting something a little different - and the changes here make it unnecessary. #408 is a much bigger issue, so should be kept open. I'm not clear on what's still needed for ESCOMP/mizuRoute#426 but I'll let others make the call on that one - so, no, we shouldn't close it.

[sjsprecious]

@sjsprecious - thank you for updating the PR description. What you had written under the answer changing section didn't seem quite right. I have edited it. Can you read my edited version of "answer changes" in the top-level PR comment and confirm that this seems correct?

Thanks @billsacks . So there are two things done in this PR now:

• We tested the difference between online mapping vs. offline mapping, and they can be bit-fo-bit as long as I provided LND2ROF_FMAPNAME for consd and LND2ROF_FRAC_FMAPNAME for consf (in my original implementation), comparing this branch to the main branch.
• We tested the same LND2ROF_FMAPNAME offline mapping for both consd and consf, and there is a roundoff difference as expected.

Thus my understanding is that:

• Using the offline mapping is bit-for-bit (which is the focus of my original implementation).
• Changing the code to use LND2ROF_FMAPNAME for both consd and consf causes the answer changing.

Your revised description seems to indicate that using the offline mapping file causes the difference and I don't think that is the case. Correct me if I misunderstood anything or I am just overinterpreting your statement here.

Regarding the issues: thanks for checking. I just closed #409 as wontfix, since it was suggesting something a little different - and the changes here make it unnecessary. #408 is a much bigger issue, so should be kept open. I'm not clear on what's still needed for ESCOMP/mizuRoute#426 but I'll let others make the call on that one - so, no, we shouldn't close it.

Thanks for your clarification. That sounds good to me.

[billsacks]

My edits of the "answer changes" section were based on the latest version of the code, which only has a single LND2ROF mapping file. Given that we have just one LND2ROF mapping file now, my understanding is that it is now impossible to get exactly bit-for-bit results between online and offline mappings, which is what I wanted to make clear. Does that sound right to you? If not, perhaps we can have a short chat to get to some shared understanding of this.

I had missed the point that your previous version allowed you to get bit-for-bit results between online and offline. I'm a little surprised that that's the case given my understanding of how the online mapping is supposed to work (because I thought that the online mapping would use a mask on the land mesh, which the offline mapping does not), but your test results do seem to support that. From my perspective, I still think it makes sense to have this single mapping file to keep things simpler from a user perspective, even though you can't reproduce results bit-for-bit between online and offline mapping files. But let me know if that is problematic for you.

[sjsprecious]

My edits of the "answer changes" section were based on the latest version of the code, which only has a single LND2ROF mapping file. Given that we have just one LND2ROF mapping file now, my understanding is that it is now impossible to get exactly bit-for-bit results between online and offline mappings, which is what I wanted to make clear. Does that sound right to you? If not, perhaps we can have a short chat to get to some shared understanding of this.

This is correct, but it is because we only have one LND2ROF mapping file as you said. We could still keep the BFB result by adding the LND2ROF_FRAC option and using the offline mapping file, but it is not what you want. Thus the root cause of answer changing here is the choice of using a single LND2ROF mapping file, not using online vs. offline mapping (to me, the later statement is suggesting that there is a problem with ESMF regrdding function done online vs. offline, but this is not the case here).

I had missed the point that your previous version allowed you to get bit-for-bit results between online and offline. I'm a little surprised that that's the case given my understanding of how the online mapping is supposed to work (because I thought that the online mapping would use a mask on the land mesh, which the offline mapping does not), but your test results do seem to support that. From my perspective, I still think it makes sense to have this single mapping file to keep things simpler from a user perspective, even though you can't reproduce results bit-for-bit between online and offline mapping files. But let me know if that is problematic for you.

In my previous test, I achieved BFB by providing separate offline mapping files to consf and consd. I agree with your statement that using a single mapping file is simpler. My point is that using offline mapping file vs. using online mapping itself is a BFB change (the original focus of my PR), while using a single LND2ROF mapping file for consf and consd is answer-changing (nothing to do with my goal of reducing memory usage, but it seems a bug fix based on your discussion above).

[billsacks]

@sjsprecious - thank you for articulating your concern with my wording. I have updated the top-level comment to try to make this more clear. Can you take a look and see if you agree with this new wording?

while using a single LND2ROF mapping file for consf and consd is answer-changing (nothing to do with my goal of reducing memory usage, but it seems a bug fix based on your discussion above).

I just want to clarify (mainly for the sake of myself or others who might come back to this discussion in the future): using a single lnd2rof mapping file is not itself really a bug fix in my mind. Rather, using just one file here instead of two separate files is a way to stop the unnecessary difference of where we use consd vs. consf mapping (see #408) from growing even more out of control - given that these two mapping types are mathematically equivalent. Your original implementation of having two separate mapping files was very reasonable given the current state of the code, but we want to stop this complexity from growing further in an attempt to eventually move us towards the long-term goal of having a single mapping type (consd) used everywhere.

[sjsprecious]

@sjsprecious - thank you for articulating your concern with my wording. I have updated the top-level comment to try to make this more clear. Can you take a look and see if you agree with this new wording?

while using a single LND2ROF mapping file for consf and consd is answer-changing (nothing to do with my goal of reducing memory usage, but it seems a bug fix based on your discussion above).

I just want to clarify (mainly for the sake of myself or others who might come back to this discussion in the future): using a single lnd2rof mapping file is not itself really a bug fix in my mind. Rather, using just one file here instead of two separate files is a way to stop the unnecessary difference of where we use consd vs. consf mapping (see #408) from growing even more out of control - given that these two mapping types are mathematically equivalent. Your original implementation of having two separate mapping files was very reasonable given the current state of the code, but we want to stop this complexity from growing further in an attempt to eventually move us towards the long-term goal of having a single mapping type (consd) used everywhere.

Thanks @billsacks . The updated PR description looks good to me and I have no more questions about it.

Thanks for your clear explanation. I think it is helpful for other people to understand the discussion here in the future.

[fischer-ncar]

The aux_cime_baselines tests passed last night. I do have 2 tests that are stuck in the queue, but I'm expecting them to pass.

[sjsprecious]

The aux_cime_baselines tests passed last night. I do have 2 tests that are stuck in the queue, but I'm expecting them to pass.

Thanks @fischer-ncar for doing the test. Actually my PR ends up with an answer-changing one so I am surprised that your tests passed. Maybe aux_cime_baseline does not have a test that touches land/rof mask? @billsacks

[billsacks]

Thanks @fischer-ncar !

@sjsprecious - we don't set the lnd2rof mapping file in typical cases - we always use the online mapping - so it seems correct that all baselines passed for our testing.

[sjsprecious]

Thanks @fischer-ncar !

@sjsprecious - we don't set the lnd2rof mapping file in typical cases - we always use the online mapping - so it seems correct that all baselines passed for our testing.

Ah I see. That makes sense. Thanks for your clarification.