-
Notifications
You must be signed in to change notification settings - Fork 104
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
Merging in changes from release/public-v1 #141
Conversation
macOS + GNU build
Add RST file for UPP documentation
Update CMakeLists.txt files to support GNU compilers
Adding support files for documentation to ufs public release branch
…_release Change version number from 8.0.0 to 1.0.0 for UFS release
correct installation directory for Fortran module files.
@WenMeng-NOAA - We have conducted regressions tests on Hera successfully for this merge. Results can be found here: /scratch2/BMC/det/KaYee/UPP/mike/cmake/ and in the log file under scripts/rt.log.HERA within that directory. With your approval on this PR, we'd like to merge this into develop first. Then have @aerorahul add any cmake changes on top of that. |
@@ -0,0 +1 @@ | |||
1.0.0 |
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.
@WenMeng-NOAA - We need to decide what this version number should in develop branch. See the discussion of this VERSION file in #140 conversation.
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.
@fossell The branch develop is the main development branch. We would create release branch e.g. release/gfs_v16,
release/public_v1 from develop when specific application is in pre-implementation. Will we add version number from
release branches and leave VERSION unfilled in branch develop?
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.
@WenMeng-NOAA That could work. I was debating whether we should leave VERSION out of develop completely, and only have it in specific application branches with corresponding version number for that application. I'm ok with either approach I think, though having an empty version file might be confusing to users/developers? This is beyond this PR, but maybe I can try to create explanation of why we don't have a straightforward versioning system within develop.
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.
@fossell I think leave VERSION out of develop would be better. VERSION can be started from a application branch.
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.
@WenMeng-NOAA - Sounds good. I will delete VERSION file from this PR.
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.
@fossell deleting this file will result in a failed build.
Changes will be required in CMakeLists.txt
.
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.
@aerorahul - The cmake build will fail, correct? Not other regular reg tests. Can you then address this with appropriate mods to CMakeLists.txt when updating your #140 after we make this merge with the current release/public-v1? This PR will be specifically for merging the current release/public-v1 as-is into develop, followed by any updates to your #140 PR to make updates to cmake accordingly. Will that work?
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.
@fossell I cannot do that. My changes are built on top of what is in the public/release-v1.
A PROJECT_VERSION
is required, so that downstream applications can "find" the package with a minimum version requirement.
The UFS depends on it.
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.
@fossell Can we use public release version number for VERSION? Right now, all other UPP versions for ops. NCEP applications haven't use CMAKE function yet.
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.
Per conversation with @WenMeng-NOAA and @aerorahul, decision was made to keep VERSION file in develop and add placeholder version number that will be changed in future after internal UPP decision is made on versioning. @aerorahul will also add comments in the VERSION file via hot fix merge to make it clear to developers and users that this is not official.
@WenMeng-NOAA On the contrary, if UPP has and maintains a versioning system, all releases of GFS, GEFS and UFS would use a single identifiable version from UPP. This is the practice followed for other software, e.g. libraries from NCEPLIBS. For e.g. bacio, nemsio, g2tmpl etc are not incremented based on GFS or GEFS release versioning. They carry their internal versioning, and GFS, GEFS etc. assembles its dependencies. |
@aerorahul During model development stage, the develop integrate changes from various models. Using unify version numbers (e.g. 8.0.1 for gfs v17, 8.0.2 for gefs v14, 8.0.3 for fv3 sar, 8.0.4 for gfs v17...) make model developers hard to track new features for their applications. The GFS, GEFS, FV3 SAR will not use same develop SHA for release. I create gfs v16 release branch when when gfs implementation manager announce the code refrozen no more scientific changes. From the code frozen to ops. implementation, any changes for bug fix, EE2 recommendation, etc. will be added in release branch and create tags with version gfsv16.v1.0.1, 1.0.2...1.0.9. The GFS V16 developers can easily track changes. |
@WenMeng-NOAA Regardless, I think having an internal versioning system for UPP should be maintained. |
@aerorahul Thanks for suggesting the semantic version system. We might discuss within UPP (EMC and DTC ) side for a better approach to support implementation and community. On the EMC side, we have limited resource. I am the person integrate changes from different developer teams , run regression tests, provide UPP support for parallel run and make final release. With a lot of inquires, we try the best get things done one by one gradually. |
@aerorahul I'll echo Wen and thank you for suggestions in how to version. This has been an ongoing discussion and we will continue to discuss internally to try to find the best solution for all involved. UPP has come a long way in the past couple years from multiple separate repositories to a more consolidated effort to name one big change, and while progress may be slow we are committed to improving the system and procedures. Thanks for your patience and understanding on this issue. |
Yes, public release version will work. But again, that is the public
release version of the UFS as a whole system. How is it related to the
internal versioning of UPP as a software? But, that is a topic for another
discussion.
…On Fri, Jun 19, 2020 at 1:50 PM WenMeng-NOAA ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In VERSION
<#141 (comment)>:
> @@ -0,0 +1 @@
+1.0.0
@fossell <https://github.com/fossell> Can we use public release version
number for VERSION? Right now, all other UPP versions for ops. NCEP
applications haven't use CMAKE function yet.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#141 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACW5YTUG6QZF4L2BGIUVFK3RXOQORANCNFSM4OCEENQQ>
.
|
@WenMeng-NOAA - Mike has restored the VERSION file to this PR. It is ready for your final approval and merge. |
@fossell Thanks for quick action. I will work on testing on different platforms. |
@fossell and @mkavulich It seems this branch has a commit (#135) behind branch develop at original UPP repos. Can you sync it? Thanks! |
@WenMeng-NOAA - Mike has updated the branch to develop. Should be ready for your final tests/approval and merge. Thanks! |
The regression tests were conducted on Dell, Cray and Hera. No changed results are found. The branch is ready for merging to develop. |
Add CODEOWNERS file with Julie, Mike, Gerard and Jeff as owners.
This is a merge of branch release/public-v1 into develop.
Only a few files had conflicts that needed to be manually resolved: sorc/ncep_post.fd/grib2_module.f and sorc/ncep_post.fd/CALRAD_WCLOUD_newcrtm.f
EMC regression tests completed successfully on Hera.
closes #38
closes #138