Adds a new tool to replace gen_domain in CIME #6347

whannah1 · 2024-04-15T22:11:29Z

"generate_domain_files_E3SM.py" is a new tool to replace the fortran
gen_domain.F90 tool that exists in CIME. The old version was problematic
to use because the build configuration was almost always broken when a
user went to build it. The new python version works efficiently due to
the numba package, which is included in the E3SM unified environment.

[BFB]

github-actions · 2024-04-15T22:13:26Z

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6347/
on branch `gh-pages` at 2024-04-24 21:05 UTC

singhbalwinder

Thanks for rewriting this tool! I left some very minor comments.

tools/generate_domain_files/generate_domain_files_E3SM.py

whannah1 · 2024-04-17T18:46:27Z

I think this PR is ready for a thorough review.

I've added documentation for this tool and a new "tools" section of the E3SM docs. I expect other tool documentaiton will show up there soon.

Additionally, I've also started working on recreating the guide for "Running E3SM on New Grids" in the new E3SM docs repo. So there will naturally be some redundant documentation for this tool, but I think that's ok.

mahf708

Flash review. Feel free to push back and/or ignore. I like the shiny new tool! I think eventually we want to have a neat conda package with all your tools 😸

mkdocs.yaml

tools/generate_domain_files/docs/index.md

mahf708 · 2024-04-17T19:02:47Z

tools/generate_domain_files/mkdocs.yaml

@@ -0,0 +1 @@
+site_name: generate_domain_files


Two comments here:

I am not sure we want this to be generate_domain_files --- maybe like the title in the file? "Generating Domain Files"?

I am not sure if we want each tool to have its sub-site like this. Maybe our strategy should be that we give "tools" their own website (like eam) but then inside tools, each additional tool will just be a page (and maybe some subpages)

Yea, I'm a bit unsure about the best strategy here as well. Initially I was envisioning a more complicated hierarchy, but then Ben Hillman pointed out that I was committing the sin of premature optimization. So I think it's best to see how the documentation evolves and what new tools get added before we make any decisions about how to reorganize things.

singhbalwinder · 2024-04-23T18:22:37Z

Hi @jonbob and @mahf708: Do you guys have any further comments on this PR before I merge it? Thanks!

mahf708 · 2024-04-23T18:24:49Z

I don't think we should have each tool get its own sub-website with mkdocs.yaml. Pinging @rljacob who added the monorepo functionality. Having said that, we can resolve this issue later (it is mostly cosmetic, nothing fundamental), so no need to hold this PR.

jonbob · 2024-04-23T18:36:58Z

@singhbalwinder - I was going to test it and compare output from the Fortran tool quickly, but I'll do that today and post results here

jonbob · 2024-04-23T18:45:02Z

@whannah1 -- it failed for me trying to regenerate a domain file from one of our recent ice/ocn meshes:

python generate_domain_files_E3SM.py -m /lcrc/group/acme/ac.jwolfe/v3-grids/RRS6to18E3r6/map_RRS6to18E3r6_to_TL319_traave.20240403.nc -o RRS6to18E3r6 -l TL319

  Input files and parameter values:
    map_file        : /lcrc/group/acme/ac.jwolfe/v3-grids/RRS6to18E3r6/map_RRS6to18E3r6_to_TL319_traave.20240403.nc
    lnd_grid        : TL319
    ocn_grid        : RRS6to18E3r6
    fminval         : 0
    fmaxval         : 1
    set_omask       : False
  

  Grid information from map file:
    domain_a file        : ocean.RRS6to18E3r6.scrip.20240402.nc
    domain_b file        : TL319.151007.nc
    ocn_grid_file        : ocean.RRS6to18E3r6.scrip.20240402.nc
    atm_grid_file        : TL319.151007.nc
    ocn grid size   (n_a): 4016094
    atm grid size   (n_b): 204800
    sparse mat size (n_s): 5992418
  
Traceback (most recent call last):
  File "/lcrc/group/acme/ac.jwolfe/e3sm_PR6347/tools/generate_domain_files/generate_domain_files_E3SM.py", line 360, in <module>
    main()
  File "/lcrc/group/acme/ac.jwolfe/e3sm_PR6347/tools/generate_domain_files/generate_domain_files_E3SM.py", line 211, in main
    add_metadata(ds_out)
  File "/lcrc/group/acme/ac.jwolfe/e3sm_PR6347/tools/generate_domain_files/generate_domain_files_E3SM.py", line 301, in add_metadata
    ds_out['xc'] = ds_out['xc'].assign_attrs({'long_name':'longitude of grid cell center'})
NameError: name 'ds_out' is not defined

Of course it's highly likely to be user error

rljacob · 2024-04-23T19:46:31Z

I agree with @mahf708. Move the docs directory to just under tools.

whannah1 · 2024-04-23T21:45:54Z

After discussing how to restructure the tools docs with @rljacob we didn't like the alternative, so we decided to leave it as is.

I also fixed the bug that @jonbob found.

whannah1 · 2024-04-23T22:40:48Z

@singhbalwinder looks like I have another problem with the output domain files that I need to investigate, so we need to hold off on merging this.

mahf708 · 2024-04-23T22:55:37Z

looks like I have another problem with the output domain files that I need to investigate, so we need to hold off on merging this.

@whannah1, not to add more work to this, but should we consider adding a unit test to go with this? Then, we can even have that unit test go in one of these actions. An example of a python-centric unit test:

gh action file: https://github.com/E3SM-Project/E3SM/blob/master/.github/workflows/eamxx_default_files.yml
which calls this piece of code: https://github.com/E3SM-Project/E3SM/blob/master/components/eamxx/cime_config/tests/eamxx_default_files.py

If we have these files on the inputdata server, it is then relatively easy to request them, etc. (as long as we are not accumulating more than ~20 GB or so, we should be able to test this pretty quickly).

I thought it would be opportune time to suggest since you're likely debugging and having a clear systematic view of the code, etc., and I am happy to put together whatever you end up assembling in a gh-ci-compliant format :)

jonbob

Approved based on testing, which showed BFB results comparing a land domain file made with this tool vs one made with the Fortran version

whannah1 · 2024-04-24T19:37:43Z

@mahf708 I want to hold off on the testing idea mainly because I'm not sure how to write a coherent test. We could set it up to just go through motions and prove that it can produce a set of files, but I'm not sure how to test the output for any sort of correctness. So let's hold off.

@singhbalwinder The issue that Jon identified is now fixed, so this PR is ready to be merged.

whannah1 · 2024-04-24T21:25:48Z

As one last test I ran a quick test using F2010 on the ne30pg2_oECv3 grid replacing the default domain files with ones produced by the new tool and it ran without issue. However, I didn't do a baseline comparison.

Adds a new tool to replace gen_domain in CIME "generate_domain_files_E3SM.py" is a new tool to replace the fortran gen_domain.F90 tool that exists in CIME. The old version was problematic to use because the build configuration was almost always broken when a user went to build it. The new python version works efficiently due to the numba package, which is included in the E3SM unified environment. [BFB] * whannah/new-gen_domain-tool: revert lfrac threshold and adjust default fminval add opts.lnd_mask_threshold for new gen domain tool remove "test" prefix for output domain files cosmetic fix misc bug fixes after PR changes switch bash to shell change yaml to yml typo fix in docs refactor domain generation tool remove un-used eps variable linter fixes update docs update docs add text to docs fix typo rename yaml file move domain tool and add documentation update tools/generate_domain_files_E3SM.py add new tool to replace gen_domain in cime

singhbalwinder · 2024-04-30T00:51:26Z

Merged to next

add new tool to replace gen_domain in cime

7d55c63

whannah1 added 5 commits April 16, 2024 08:34

update tools/generate_domain_files_E3SM.py

1d67d94

move domain tool and add documentation

ede39e5

rename yaml file

3573055

fix typo

2930e54

add text to docs

75fcd82

rljacob requested review from jonbob and bishtgautam April 17, 2024 01:58

rljacob assigned singhbalwinder Apr 17, 2024

rljacob added the inputdata Changes affecting inputdata collection on blues label Apr 17, 2024

singhbalwinder approved these changes Apr 17, 2024

View reviewed changes

tools/generate_domain_files/generate_domain_files_E3SM.py Outdated Show resolved Hide resolved

tools/generate_domain_files/generate_domain_files_E3SM.py Outdated Show resolved Hide resolved

whannah1 added 6 commits April 17, 2024 11:45

update docs

829f56a

update docs

450dee1

linter fixes

d97f0b2

remove un-used eps variable

bc65314

refactor domain generation tool

e851050

typo fix in docs

0bf7561

whannah1 changed the title ~~[WIP] add new tool to replace gen_domain in CIME~~ add new tool to replace gen_domain in CIME Apr 17, 2024

mahf708 reviewed Apr 17, 2024

View reviewed changes

whannah1 added 2 commits April 17, 2024 13:13

change yaml to yml

be2735c

switch bash to shell

fdc6927

bishtgautam approved these changes Apr 23, 2024

View reviewed changes

misc bug fixes after PR changes

31703ab

cosmetic fix

531043f

remove "test" prefix for output domain files

b2a5af7

add opts.lnd_mask_threshold for new gen domain tool

e56c1bc

whannah1 force-pushed the whannah/new-gen_domain-tool branch from 94a1882 to e56c1bc Compare April 24, 2024 17:14

revert lfrac threshold and adjust default fminval

b66d43e

jonbob approved these changes Apr 24, 2024

View reviewed changes

singhbalwinder changed the title ~~add new tool to replace gen_domain in CIME~~ Adds a new tool to replace gen_domain in CIME Apr 30, 2024

singhbalwinder merged commit 9ee9e1d into master Apr 30, 2024
14 checks passed

singhbalwinder deleted the whannah/new-gen_domain-tool branch April 30, 2024 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a new tool to replace gen_domain in CIME #6347

Adds a new tool to replace gen_domain in CIME #6347

whannah1 commented Apr 15, 2024 •

edited by singhbalwinder

Loading

github-actions bot commented Apr 15, 2024 •

edited

Loading

singhbalwinder left a comment

whannah1 commented Apr 17, 2024

mahf708 left a comment

mahf708 Apr 17, 2024

whannah1 Apr 17, 2024

singhbalwinder commented Apr 23, 2024

mahf708 commented Apr 23, 2024

jonbob commented Apr 23, 2024

jonbob commented Apr 23, 2024

rljacob commented Apr 23, 2024

whannah1 commented Apr 23, 2024

whannah1 commented Apr 23, 2024

mahf708 commented Apr 23, 2024

jonbob left a comment

whannah1 commented Apr 24, 2024

whannah1 commented Apr 24, 2024

singhbalwinder commented Apr 30, 2024

Adds a new tool to replace gen_domain in CIME #6347

Adds a new tool to replace gen_domain in CIME #6347

Conversation

whannah1 commented Apr 15, 2024 • edited by singhbalwinder Loading

github-actions bot commented Apr 15, 2024 • edited Loading

singhbalwinder left a comment

Choose a reason for hiding this comment

whannah1 commented Apr 17, 2024

mahf708 left a comment

Choose a reason for hiding this comment

mahf708 Apr 17, 2024

Choose a reason for hiding this comment

whannah1 Apr 17, 2024

Choose a reason for hiding this comment

singhbalwinder commented Apr 23, 2024

mahf708 commented Apr 23, 2024

jonbob commented Apr 23, 2024

jonbob commented Apr 23, 2024

rljacob commented Apr 23, 2024

whannah1 commented Apr 23, 2024

whannah1 commented Apr 23, 2024

mahf708 commented Apr 23, 2024

jonbob left a comment

Choose a reason for hiding this comment

whannah1 commented Apr 24, 2024

whannah1 commented Apr 24, 2024

singhbalwinder commented Apr 30, 2024

whannah1 commented Apr 15, 2024 •

edited by singhbalwinder

Loading

github-actions bot commented Apr 15, 2024 •

edited

Loading