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

Pre-release improvements: remove unnecessary JASPER check, check that CRTM exists in configure script #53

Merged
merged 2 commits into from
Jan 23, 2020

Conversation

mkavulich
Copy link
Contributor

This is a variety of fixes needed prior to the stand-alone UPP release.

  • Remove check in configure/compile system for jasper libraries; this was only necessary when we were building our own copies of the NCEP libraries
  • Add check for missing CRTM; the message may need to be modified depending on how we decide to release the code to the community
  • Remove and rename files to get around the compilation issue for case-insensitive operating systems. This has not yet been tested for the NCEP build but it should work.

Copy link
Contributor

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Looks ok to me! Don't think that any of this other than checking for the CRTM/JasPER requirements will be needed for the ufs_release_v1.0 branch, because the cmake build is out of source. But I will check.

@fossell
Copy link
Contributor

fossell commented Jan 23, 2020

@climbfuji - This PR is for DTC_post branch, which is our community standalone release (not ufs release). However, our goal if possible is have DTC_post and ufs branch be identical for both releases. From your comment it sounds like for ufs build that cmake only cares about the sorc/ncep_post.fd directory, is that correct? If so, then a merge of DTC_post and ufs_public_release should be no problem, agreed? As background, DTC_post has some directory and build structure changes to it to help facilitate standalone community portability. These changes are not yet in develop branch, but will be eventually. Note that we are still finishing up final tests for DTC_post merged with develop, so DTC_post source still appears behind develop (and ufs branch) on github right now. We hope to have DTC_post updated in the very near future. Let me know your thoughts on the potential merge of DTC_post and ufs_public_release (and if we're still using ufs_public_release branch or if that has changed again to ufs_releave_v1.0 as you mentioned in your comment). If DTC_post and ufs release branches are identical, or very close, that will be a huge help to the DTC support team as we try to tackle support both. Thanks!

@climbfuji
Copy link
Contributor

I agree, we should try to keep them as close as possible, given that you are planning to update develop from DTC_post. The branch names are still confusing to me. I remember you were asked to change ufs_release_v1.0 to ufs_public_release. Now EMC_post = NCEPLIBS-post is the only repo that uses ufs_public_release instead of ufs_release_v1.0 among all the NCEPLIBS.

I am tracing a segfault of the ufs_public_release code on macOS. Since Mike used the DTC_post branch successfully on macOS, I am looking at differences between the two (although the reason may be something else, but need to start somewhere).

@fossell
Copy link
Contributor

fossell commented Jan 23, 2020

@climbfuji Sounds good, thanks! I'll try to keep ufs_release_v1.0 and ufs_public_release in sync as we move along, until someone makes the final call on naming convention.

@fossell
Copy link
Contributor

fossell commented Jan 23, 2020

@mkavulich this is ok with me, however note that there are some mods in develop we need to bring into DTC_post (bug fixes and final decisions with EMC that went into develop but not DTC_post in the past couple months due to all of the chaos). E.g., RQSTFLD.f has been moved to .F file since we added compiler directives to make our last standalone release portable to community with grib1 support. So your change to FTN suffix will need to be applied to that when we merge with develop as well. Other bug fixes are important too. Do you think we should finalize testing on DTC_post merge with develop first and do that commit, then commit this PR after?
@climbfuji - not sure if you're using DTC_post as-is or just trying this suffix fix for macos within the ufs release branch, but just FYI that if something fails with DTC_post, we're still actively working on it and really need develop merged before I'd consider it stable and portable.

@climbfuji
Copy link
Contributor

I don't think the case-sensitivity changes are need for ufs_public_release - the cmake build is out of source and should not suffer from this problem.

@mkavulich
Copy link
Contributor Author

@fossell I really should break this change into two: one with the case-sensitivity changes, and one with the Jasper change and the rest. Mainly because I think we should do the jasper changes first (so we can run the UFFDA properly) and then we can make the case-sensitivity changes later. I can do that pretty easily once you give me the go-ahead.

@fossell
Copy link
Contributor

fossell commented Jan 23, 2020

@mkavulich This makes sense to me, thanks!

@mkavulich mkavulich force-pushed the pre-release-cleanup branch from 450c4a9 to 3c0f304 Compare January 23, 2020 21:37
@mkavulich mkavulich changed the title Pre-release improvements: remove unnecessary JASPER check, fix case-sensitivity problems Pre-release improvements: remove unnecessary JASPER check, check that CRTM exists in configure script Jan 23, 2020
@mkavulich
Copy link
Contributor Author

@fossell Okay, this pull request can be merged now, I have pulled out the case-sensitivity changes and will open another pull request for that

@fossell fossell merged commit e241779 into NOAA-EMC:DTC_post Jan 23, 2020
guoqing-noaa pushed a commit to guoqing-noaa/UPP that referenced this pull request Apr 28, 2022
EricJames-NOAA referenced this pull request in EricJames-NOAA/UPP Dec 14, 2022
…iles_ics

Split get_extrn_mdl_files.sh into j-job and ex-script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants