Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move plumber2 scripts to python directory (on b4b branch) #2505

Merged
merged 2 commits into from
May 2, 2024

Conversation

TeaganKing
Copy link
Contributor

@TeaganKing TeaganKing commented May 1, 2024

Description of changes

This PR addresses #2187 by moving new scripts that were added in #2155 to tools/site_and_regional into the python directory, similarly to other NEON scripts in #2156.

Fixed: #2187

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any:
I tested newly added unit test, which was successful, with python -m unittest test_unit_plumber2_surf_wrapper.py. I will add that when testing with ./run_ctsm_py_tests -u, I ran into a seemingly unrelated error that I'll add in a comment below.

@TeaganKing
Copy link
Contributor Author

TeaganKing commented May 1, 2024

The unit tests, ./run_ctsm_py_tests -u, run cleanly on derecho, but not on casper (see failure related to the 'craype' module in comment on other PR).

The sys tests, run with ./run_ctsm_py_tests -s, has some errors with pre-existing tests on both casper (related to CIME?) and derecho (cartopy not found & CESMDATAROOT env var error).

Since this PR just moves two minimally integrated scripts and one test (which passes), I'd be really surprised if it is causing these failures.

@slevis-lmwg
Copy link
Contributor

@TeaganKing these days we make sure the python tests pass on derecho, so I'm not worried about problems on casper.

Are you on ctsm5.2.001 (when you type git describe)? I happen to be on ctsm5.2.002 (should not matter relative to 001) but the python tests pass for me...

@slevis-lmwg
Copy link
Contributor

I will check out your branch and see if I get the same errors that you get.

@TeaganKing
Copy link
Contributor Author

TeaganKing commented May 1, 2024

Hi @slevis-lmwg I'm on ctsm5.2.001-227-g9724557ae (I just made a branch this morning off of b4b-dev)

@slevis-lmwg
Copy link
Contributor

I will check out your branch and see if I get the same errors that you get.

For me git describe is now the same as yours ctsm5.2.001-227-g9724557ae and

./run_ctsm_py_tests -u
./run_ctsm_py_tests -s

passed. So I suspect that you're dealing with an environment issue.

@TeaganKing
Copy link
Contributor Author

I will check out your branch and see if I get the same errors that you get.

For me git describe is now the same as yours ctsm5.2.001-227-g9724557ae and

./run_ctsm_py_tests -u
./run_ctsm_py_tests -s

passed. So I suspect that you're dealing with an environment issue.

Ok, great! I'm glad to hear that's working for you!

@TeaganKing TeaganKing added PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete bfb bit-for-bit labels May 1, 2024
@slevis-lmwg
Copy link
Contributor

Once approved, is this something that you can merge to b4b-dev yourself @TeaganKing, or do you need me to do it?

Copy link
Contributor

@slevis-lmwg slevis-lmwg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things look good. I pulled the latest commit in my copy of this branch. I'm rerunning the python tests and will also run the clm_pymods test-suite.

I'm approving, though let's not merge until I complete these tests.

@TeaganKing
Copy link
Contributor Author

I'm looking at the CTSM Dev Branch document instructions for merging to b4b-dev. I'm happy to go through this, as long as permissions are fine to do so, and I may also need help with the aux_clm testing.

@slevis-lmwg
Copy link
Contributor

As before, the unit and system tests passed.

Given that modifications are limited to python codes, let's just do the clm_pylib test-suite, which is much quicker. I'm happy to do it. It may also be better if you do it for the practice, so let me know if you need help.

@TeaganKing
Copy link
Contributor Author

Hi @slevis-lmwg , I'm also not sure that I've run the clm_pylib tests? Is this different from the run_ctsm_py_tests testing? Could you direct me towards how to run this?

@slevis-lmwg
Copy link
Contributor

From your branch's /ctsm directory (which you likely gave some other name, so I mean the directory just one up from /python), assuming that you have run ./manage_externals/checkout_externals, you then run
./run_sys_tests -s clm_pymods -c ctsm5.2.001 --skip-generate
This is just like when you run aux_clm. For b4b-dev we compare to the baseline but don't generate a new baseline. I hope this works ok.

@TeaganKing
Copy link
Contributor Author

TeaganKing commented May 1, 2024

Thanks for these directions! Here's the result that I got. It looks like ./doc/doc-builder is marked with e-o (but says not checked out, so I'm not sure if this is an issue?) and ./src/fates is marked with s (skipped?)

Testroot: /glade/derecho/scratch/tking/tests_0501-164700de

create_test --retry: 0

Current hash: 938be5fe1 (Teagan King, Wed May 1 14:05:41 2024 -0600) remove the old files that were moved to a new directory
## move_plumber_scripts...origin/move_plumber_scripts
(clean sandbox)
------------------------------------------------------------------------
manage_externals status:
Processing externals description file : Externals.cfg (/glade/work/tking/ctsm_2187/CTSM)
Processing externals description file : Externals_CLM.cfg (/glade/work/tking/ctsm_2187/CTSM)
Processing externals description file : Externals_CISM.cfg (/glade/work/tking/ctsm_2187/CTSM/components/cism)
Processing externals description file : .gitmodules (/glade/work/tking/ctsm_2187/CTSM/cime)
Processing externals description file : Externals_CDEPS.cfg (/glade/work/tking/ctsm_2187/CTSM/components/cdeps)
Checking local status of required & optional components: clm, fates, cism, source_cism, rtm, mosart, mizuroute, ccs_config, cime, cime/non_py/cprnc, cmeps, cdeps, fox, genf90, cpl7, share, mct, parallelio, doc-builder,
    ./ccs_config
        clean sandbox, on ccs_config_cesm0.0.92
    ./cime
        clean sandbox, on cime6.0.217_httpsbranch03
    ./cime/CIME/non_py/cprnc
        clean sandbox, on v1.0.5
    ./components/cdeps
        clean sandbox, on cdeps1.0.28
    ./components/cdeps/fox
        clean sandbox, on 4.1.2.1
    ./components/cdeps/share/genf90
        clean sandbox, on genf90_200608
    ./components/cism
        clean sandbox, on cismwrap_2_1_99
    ./components/cism/source_cism
        clean sandbox, on cism_main_2.01.013
    ./components/cmeps
        clean sandbox, on cmeps0.14.50
    ./components/cpl7
        clean sandbox, on cpl77.0.7
    ./components/mizuRoute
        clean sandbox, on 34723c2e4df7caa16812770f8d53ebc83fa22360
    ./components/mosart
        clean sandbox, on mosart1_0_49
    ./components/rtm
        clean sandbox, on rtm1_0_79
e-o ./doc/doc-builder
        -, not checked out --> v1.0.8
    ./libraries/mct
        clean sandbox, on MCT_2.11.0
    ./libraries/parallelio
        clean sandbox, on pio2_6_2
    ./share
        clean sandbox, on share1.0.18
s   ./src/fates
        clean sandbox, sci.1.73.0_api.35.0.0 --> sci.1.72.2_api.34.0.0

@slevis-lmwg
Copy link
Contributor

I think that's fine. Now you can check that the tests are running using qstat -u <your_username> and when they're done you'll cd into the tests_0501... directory that should now already be in your /ctsm directory and run ./cs.status.fails to see if anything failed.

@TeaganKing
Copy link
Contributor Author

Ok. It looks like there were some SETUP failures. I'm not entirely sure what the normal output is here?

0501-164700de_gnu: 1 test
    FAIL FSURDATMODIFYCTSM_D_Mmpi-serial_Ld1.5x5_amazon.I2000Clm50SpRs.derecho_gnu SETUP


0501-164700de_int: 2 tests
    FAIL LILACSMOKE_D_Ld2.f10_f10_mg37.I2000Ctsm50NwpSpAsRs.derecho_intel.clm-lilac SETUP
    FAIL MKSURFDATAESMF_P128x1.f10_f10_mg37.I1850Clm50BgcCrop.derecho_intel SETUP


========================================================================
Non-PASS results for select phases:
TPUTCOMP non-passes: 0
MEMCOMP non-passes: 0

@slevis-lmwg
Copy link
Contributor

I will try the same test-suite in case we're up against an environment issue again.

@TeaganKing
Copy link
Contributor Author

Ok, thanks Sam! Sorry to make things more complicated!

@slevis-lmwg
Copy link
Contributor

Yay, the test-suite passed for me (so yours may be the same environment issue...)

Next, does the "merge pull request" button appear green for you? If so, then I think you have the right permissions. Otherwise, I can complete the merge.

@TeaganKing
Copy link
Contributor Author

Merging is blocked for me... Can you merge it after all? Or if you want to update my permissions, I can click the button.

@slevis-lmwg slevis-lmwg merged commit 40b6a3c into ESCOMP:b4b-dev May 2, 2024
2 checks passed
@slevis-lmwg slevis-lmwg deleted the move_plumber_scripts branch May 2, 2024 15:26
@ekluzek
Copy link
Collaborator

ekluzek commented May 4, 2024

NOTE: I'm sending stuff on a weekend I don't expect a response until you get a chance during the workweek.

@TeaganKing just a note the "s" from manage externals means that fates wasn't synced at the correct version. For this case that probably doesn't matter since you aren't running with fates. And the "o" means optional so that doc-builder wasn't checked out which is also fine.

How this works will be changing in a week or so. But it's good to recognize the states of the externals, because it does matter for other externals like cime.

Take care. Nice to see that this made it in!

@TeaganKing
Copy link
Contributor Author

Hi @ekluzek , thanks for explaining the states of the externals and what those additional letters mean!

I'm glad to see this got merged in-- thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move new PLUMBER2 scripts to python directory to enable python testing
3 participants