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

Fix subset_data error "no attribute 'evenly_split_cropland'" #2253

Merged
merged 61 commits into from
Dec 12, 2023

Conversation

slevis-lmwg
Copy link
Contributor

@slevis-lmwg slevis-lmwg commented Nov 15, 2023

Description of changes

This command returned the error:
./subset_data point --lat 38.9375 --lon 253.0625 --site Gothic --create-surface --create-user-mods --create-datm --outdir /glade/scratch/slevis/nldas/gothic --datm-syr 2000 --datm-eyr 2014 --dompft 1

Adding the following lines to subset_data.py fixes the error:

@@ -149,6 +149,13 @@ def get_parser():
         dest="cap_saturation",
         required=False,
     )
+    pt_parser.add_argument(
+        "--evenly_split_cropland",
+        help="???",
+        action="store_true",
+        dest="evenly_split_cropland",
+        required=False,
+    )

@ekluzek you plan to open an issue for resolving pylint suggestions that have appeared in recent tags and have been ignored. I could resolve these in this PR if you agree.

Update: #2253 consists of two commits (9e0f81c d895c6e), and I may have confused things by merging several other branches to this PR's branch as part of a larger BFB merge. Details below.

Specific notes

Contributors other than yourself, if any:
@wwieder encountered the error
Several additional contributors as part of the larger BFB merge.

CTSM Issues Fixed (include github issue #):
No issue opened

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:
Python testing missed this and was passing and continues to pass

glemieux and others added 30 commits July 26, 2023 19:29
This also caps the numba environment to avoid
a numba/numpy compatability issue that is
currently unaddressed by numba
FATES API update to facilitate fates refactor

This updates a number of FATES type names and module use statements
which correspond with a refactoring effort that moves FATES
patches and cohorts into their own respective modules.

With the FATES update is a minor science update, so there are
changes to answers for FATES.

This also incorporates a minor update to a more recent version
of the ccs config external.
b4b changes to Python scripts, history lists, tech note, and clm_time_manager.
@slevis-lmwg
Copy link
Contributor Author

Thank you for this explanation, @ekluzek. Makes sense.

Resolved conflicts:
.git-blame-ignore-revs
This reverts commit bb536ed.
First of 3 expected reverts.
…o pass"

This reverts commit ff02bad.
Second of 3 expected reverts.
…to subset_data_fix"

This reverts commit f47de1d, reversing
changes made to f335221.

Resolve conflicts:
.git-blame-ignore-revs
@slevis-lmwg
Copy link
Contributor Author

I did 3 git reverts to remove #2080 from this group merge. All three touch python codes only.
Python testing PASS
Also "make lint" returns to the shorter list of pylint complaints.

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Dec 11, 2023

To address the above izumi test-failure...

  1. Same test from vanilla dev158 compared to baseline gave the same failure, so
    remove this test's baseline, generate new baseline, and compare to dev157:
    Ok, but comparison to dev157 says "some baseline files missing"
  2. Compare and generate same test from the dev159 branch (DONE):
    ./create_test ERP_D_Ld5_P48x1.f10_f10_mg37.I1850Clm51Bgc.izumi_nag.clm-ciso -c /fs/cgd/csm/ccsm_baselines/ctsm5.1.dev158 -g /fs/cgd/csm/ccsm_baselines/ctsm5.1.dev159

Similar solution for the 3 missing dev158 derecho tests, which are new in dev 158 (DONE).
From vanilla dev158:

./create_test SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158
./create_test ERI_D.ne30pg3_t232.I1850Clm51BgcCrop.derecho_intel.clm-clm51cam6LndTuningMode -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158
./create_test ERP_D_Ld9.ne30pg3_t232.I1850Clm51BgcCrop.derecho_intel.clm-clm51cam6LndTuningMode -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158

From branch that will be dev159:

./create_test SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev159 -c /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158
./create_test ERI_D.ne30pg3_t232.I1850Clm51BgcCrop.derecho_intel.clm-clm51cam6LndTuningMode -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev159 -c /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158
./create_test ERP_D_Ld9.ne30pg3_t232.I1850Clm51BgcCrop.derecho_intel.clm-clm51cam6LndTuningMode -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev159 -c /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158

Update the group permissions for dev159 baseline directories DONE

If all goes smoothly with these, then talk to @ekluzek about another PR that may join the merge.

@ekluzek will look into the other two failing tests. May need to open issue and deal with later. In that case add EXPECTED FAILURE.

Then I will finish the merge.

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 11, 2023

In looking at the DAE test, it passed for me and doesn't show the same error you see. And I looked at your case and see what the failure looks like. I'll try running with vanilla ctsm5.1.dev158 and see if that's an issue.

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 11, 2023

The issue with SMS_D.f10_f10_mg37.I2000Clm51BgcCrop.derecho_nvhpc.clm-crop, is just that the entry in Expected fails lists it incorrectly as failing in RUN when it should be SHAREDLIB_BUILD.

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 11, 2023

The DAE test passes for me also in vanilla ctsm5.1.dev158.

PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv CREATE_NEWCASE
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv XML
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv SETUP
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv SHAREDLIB_BUILD time=139
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv MODEL_BUILD time=48
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv SUBMIT
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv RUN time=287
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv COMPARE_base_da
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv MEMLEAK insufficient data for memleak test
PASS DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv SHORT_TERM_ARCHIVER

@slevis-lmwg maybe also try it in vanilla ctsm5.1.dev158 and see if it was just a glitch for you that it failed? There might be environment issue as well, but I don't know what it would be...

@slevis-lmwg
Copy link
Contributor Author

Ok, submitted:
./create_test DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158

@slevis-lmwg

This comment was marked as outdated.

@ekluzek

This comment was marked as outdated.

@slevis-lmwg
Copy link
Contributor Author

Resubmitted DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv this morning.
It worked, so previous failures were glitches presumably.
I hid some corresponding posts to reduce clutter.

I submitted DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv
first from my branch and second from vanilla dev158, but I think the order doesn't matter...
From branch PASS:
./create_test DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158
From vanilla dev158 PASS:
./create_test DAE_C2_D_Lh12.f10_f10_mg37.I2000Clm50BgcCrop.derecho_intel.clm-DA_multidrv -g /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev159 -c /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev158

@ekluzek this PR is now ready for the merge. Do you prefer that I wait for that additional PR that you said may fit well with this PR? Will that require a new round of testing?

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 12, 2023

@slevis-lmwg I'm having trouble getting a set of externals that work together and fix the mpi-serial intel-DEBUG issue. There is a simple change that fixes the FUNIT issue. But go ahead and make the tag as is, and we'll get that fix in later. It would've needed you to redo the testing (since enough externals had to be updated) anyway, which isn't a good way to go anyway.

@slevis-lmwg slevis-lmwg merged commit 03246f8 into ESCOMP:master Dec 12, 2023
2 checks passed
@slevis-lmwg slevis-lmwg deleted the subset_data_fix branch December 12, 2023 18:22
@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Mar 21, 2024
@samsrabin samsrabin added simple bfb bit-for-bit labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit code health improving internal code structure to make easier to maintain (sustainability) PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations
Projects
Status: Done (non release/external)
Development

Successfully merging this pull request may close these issues.

5 participants