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

case.build --clean lnd and --clean-depends lnd don't work for shared lnd lib #3286

Closed
billsacks opened this issue Nov 1, 2019 · 2 comments · Fixed by #3315
Closed

case.build --clean lnd and --clean-depends lnd don't work for shared lnd lib #3286

billsacks opened this issue Nov 1, 2019 · 2 comments · Fixed by #3315
Assignees
Labels
Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: CIMElib ty: Bug

Comments

@billsacks
Copy link
Member

When running ./case.build --clean lnd or ./case.build --clean-depends lnd, the land build is not cleaned if using a shared land build (as is standard for CESM). It looks like the problem is that COMP_LND isn't passed to the make command if you're doing a clean, and some Makefile logic depends on that in order to determine where the land build resides.

Steps to reproduce:

  1. Create a case in CESM that includes CLM (e.g., an I compset; e.g., SMS_D.f10_f10_musgs.I2000Clm50BgcCropQianRsGs.cheyenne_gnu)

  2. Run ./case.build if not already done

  3. Run ./case.build --clean lnd and/or ./case.build --clean-depends lnd.

  4. Observe that the relevant build files have NOT been removed (e.g., somewhere like bld/gnu/mpich/debug/nothreads/mct/mct/noesmf/clm/obj; I'm not sure if that's exactly the right path on cheyenne - e.g., the mpich might be something else)

  5. Optionally, in the Makefile, introduce the following diffs to further observe the issue:

    @@ -991,6 +991,7 @@ clean_dependsesp:
            $(RM) -f $(EXEROOT)/esp/obj/Srcfiles
    
     clean_dependslnd:
    +       @echo $(COMP_LND) $(USE_SHARED_CLM) $(LNDOBJDIR) >> ~/makeout
            $(RM) -f $(LNDOBJDIR)/Srcfiles
    
     clean_dependscsmshare:

    then rerun ./case.build --clean-depends lnd and take a look at the contents of ~/makeout: you should see that COMP_LND is an empty string, USE_SHARED_CLM is FALSE, and LNDOBJDIR points to the location that's used in the non-shared-build case.

I started to look into how to fix this, but wasn't sure exactly how to proceed. My first thought was to add COMP_LND to the _CMD_ARGS_FOR_BUILD variable set near the top of build.py:

_CMD_ARGS_FOR_BUILD = \
("CASEROOT", "CASETOOLS", "CIMEROOT", "COMP_INTERFACE",
"COMPILER", "DEBUG", "EXEROOT", "INCROOT", "LIBROOT",
"MACH", "MPILIB", "NINST_VALUE", "OS", "PIO_VERSION",
"SHAREDLIBROOT", "SMP_PRESENT", "USE_ESMF_LIB", "USE_MOAB",
"CAM_CONFIG_OPTS", "COMPARE_TO_NUOPC", "HOMME_TARGET",
"OCN_SUBMODEL", "CISM_USE_TRILINOS", "USE_ALBANY", "USE_PETSC")

But then I noticed that, when doing the build, these COMP values are passed in via the environment:

os.environ["COMP_{}".format(comp_class)] = comp

So that made me wonder if the preferred method would be to add the necessary environment variables when calling case.build with the --clean or --clean-depends argument.

@jedwards4b or @jgfouca can you provide any guidance here?

@billsacks billsacks added ty: Bug Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: CIMElib labels Nov 1, 2019
@billsacks
Copy link
Member Author

Side note: given how frequently issues arise with the shared land build: Long-term, I am tempted to suggest getting rid of this mechanism, and instead expanding the mechanism that @jgfouca has started to put in place to share the entire executable between tests. We would need to think more about how to leverage this from our test suites, to set up build subsets that can use a single build (because we can't get away with just one build: we need separate builds for debug vs. non-debug and a few other attributes).

@billsacks
Copy link
Member Author

billsacks commented Nov 13, 2019

From CIME telecon: preferred way for anything new is to use _CMD_ARGS_FOR_BUILD. (It should be fine to pass COMP_LND in both ways as long as they're consistent.) I'll do that.

@billsacks billsacks self-assigned this Nov 13, 2019
billsacks added a commit to billsacks/cime that referenced this issue Nov 26, 2019
Besides just passing COMP_LND (as was done in the previous commit), we
also need to reference the correct path for './case.build --clean
lnd' (the previous commit was sufficient to fix '--clean-depends lnd'
but not '--clean lnd').

Resolves ESMCI#3286
jedwards4b added a commit that referenced this issue Nov 26, 2019
Fix clean build for CLM
Fixes so that ./case.build --clean-depends lnd and ./case.build --clean lnd work for cases with CLM, whose build is in the shared build area.

Test suite: scripts_regression_tests in progress on cheyenne (I'll
update the PR if there are failures). Also:

    In a case with CLM
    (SMS_D_Ld1_P4x1.f10_f10_musgs.I2000Clm50BgcCropQianRsGs.bishorn_gnu.clm-default):
        ./case.build --clean-depends lnd
        Observed that Srcfiles was removed from the right place
        ./case.build --clean lnd
        Observed that all files were moved from the clm/obj directory in
        the shared build space

    Similar testing for SMS_D.f09_g17_gl4.T1850G.bishorn_gnu, which uses
    dlnd, and thus does NOT use the shared land space

Test baseline: N/A
Test namelist changes: none
Test status: bit for bit

Fixes #3286

User interface changes?: N

Update gh-pages html (Y/N)?: N

Code review:
jedwards4b pushed a commit that referenced this issue Dec 4, 2019
Besides just passing COMP_LND (as was done in the previous commit), we
also need to reference the correct path for './case.build --clean
lnd' (the previous commit was sufficient to fix '--clean-depends lnd'
but not '--clean lnd').

Resolves #3286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: CIMElib ty: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant