-
Notifications
You must be signed in to change notification settings - Fork 177
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
Update the gdas.cd hash and enable GDASApp to run on WCOSS2 #3220
Update the gdas.cd hash and enable GDASApp to run on WCOSS2 #3220
Conversation
This PR is opened in draft mode until g-w CI has been run on WCOSS2, Hera, Hercules, and Orion. The g-w team is invited to review and comment on changes to |
No issues here with the hash update. |
env/WCOSS2.env
Outdated
# Add path to GDASApp libraries | ||
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${HOMEgfs}/sorc/gdas.cd/build/lib" | ||
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/opt/cray/pe/mpich/8.1.19/ofi/intel/19.0/lib" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine for now, but will not be acceptable for implementation. I hope there is a more robust solution than this by that time.
More importantly, this has an impact on every executable in every job -- not just GDASApp executables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completely agree. I strongly dislike these two lines. They are temporary patches to allow GFS v17 testing and development to continue on WCOSS2.
The line
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:${HOMEgfs}/sorc/gdas.cd/build/lib"
was added because craype/2.7.17
adds
-static-libgcc -static-libstdc++ -Bstatic -lstdc++ -Bdynamic -lm -lpthread
to the ftn
command. GDASApp executables failed because they could not find JEDI libraries. Might the addition of a GDASApp install option (something we must have) resolve this problem?
Another concern with the added ftn
options is the following warning found in build_gdas.log
icpc: warning #10315: specifying -lm before files may supersede the Intel(R) math library and affect performance
ifort: warning #10315: specifying -lm before files may supersede the Intel(R) math library and affect performance
It would be unfortunate if default compiler options resulted in degraded code performance.
The line
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/opt/cray/pe/mpich/8.1.19/ofi/intel/19.0/lib"
was recommended by GDIT. GDASApp testing identified inconsistencies in across system modules. Some GDASApp executables failed with undefined symbol messages for mpi routines. GDIT is working on a solution.
g-w CI
All jobs in all streams successfully ran to completion on all machines except for C48mx500_hybAOWCDA job gdas_marineanlletkf on Hercules. Jobs from all other stream successfully ran to completion on Hercules. Only a single job from C48mx500_hybAOWCDA failed. Investigation of the failure indicates a missing job dependency in the experiment xml. A rewind and rerun resulted in successful completion of the job and, as a result, the entireC48mx500_hybAOWCDA stream. @guillaumevernieres and @AndrewEichmann-NOAA have been contacted. See issue #3219 for additional details. |
This PR is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the comments on LD_LIBRARY_PATH affecting non-gdasapp executables.
Acknowledging that gdasapp is now using space-stack on wcoss but other components are not.
ce98ffc
@aerorahul , incorporating @guillaumevernieres suggestion dismissed your approval. |
g-w issue #3222 has been opened to report the missing job dependency for C48mx500_hybAOWCDA gdas_marineanlletkf. @AndrewEichmann-NOAA will follow up on this item. |
NCO confirmed that it is OK for GDASApp to use
We can not build GDASApp using
We will stick with |
g-w CI summary CI-Hera-Passed, CI-Orion-Passed, CI-Wcoss2-Passed labels can be applied to this PR. Tests were manually run. CI-Hercules-Passed applies to all cases except C48mx500_hybAOWCDA. Testing discovered a missing job dependency for marine_gdasanlletkf. g-w issue #3222 has been opened to track resolution of this problem. All other g-w CI cases passed on Hercules. |
Thank you @WalterKolczynski-NOAA |
Description
This PR does the following:
sorc/gdas.cd
hash to bring new GDASApp functionality into g-wenv/WCOSS2.env
ush/module-setup.sh
The change to
WCOSS2.env
is due to changes introduced during the fall 2024 WCOSS2 upgrade. The change tomodule-setup.sh
is required when using spack-stack on WCOSS2.Resolves #3219
Resolves #3100
Type of change
gdas.cd
hash)Change characteristics
sorc/gdas.cd
hash. No PRs are pending.How has this been tested?
Checklist