CCPPv3 scidoc update for release#271
Closed
mzhangw wants to merge 16 commits into
Closed
Conversation
… point precision then for other compiler flags, similar to what was done for the optimization flags; fix settings additional flags for auto-generated caps by appending them
…ion_flags_includes_mynnpbl_bugfix Cmakelists separate precision flags includes mynnpbl bugfix
Collaborator
|
I assume we want a similar PR for the "master" branch afterwards, too? That one going into master will have to be held back and updated later, because the Vlab master updates are of higher priority. This PR here for the release can be reviewed and merged, if approved. |
Collaborator
|
Is the html rendering at
https://dtcenter.org/gmtb/users/ccpp/docs/sci_doc_v3_pre/ up to date with
this PR? That would be useful for us to review. Tks
…On Fri, Jun 14, 2019 at 10:25 AM Dom Heinzeller ***@***.***> wrote:
I assume we want a similar PR for the "master" branch afterwards, too?
That one going into master will have to be held back and updated later,
because the Vlab master updates are of higher priority. This PR here for
the release can be reviewed and merged, if approved.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#271?email_source=notifications&email_token=AE7WQARTN7R7Z6UY7ZW2EPTP2PA7HA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXI76Y#issuecomment-502173691>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE7WQAQXQ4XND5AJLOQLHCTP2PA7HANCNFSM4HYJIXRA>
.
|
Contributor
Author
|
Here is the latest: |
Contributor
Author
|
I am running RTs using master. Will submit PR later.
…On Fri, Jun 14, 2019 at 10:25 AM Dom Heinzeller ***@***.***> wrote:
I assume we want a similar PR for the "master" branch afterwards, too?
That one going into master will have to be held back and updated later,
because the Vlab master updates are of higher priority. This PR here for
the release can be reviewed and merged, if approved.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#271?email_source=notifications&email_token=AG7TW2SYIRQAP7OXVMLNGHTP2PA7HA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXI76Y#issuecomment-502173691>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG7TW2W4SKTTJOFZSV3E2E3P2PA7HANCNFSM4HYJIXRA>
.
|
Collaborator
|
Ok, you will have to update your code and rerun the regression tests after the Vlab master updates went in, sorry.
… On Jun 14, 2019, at 10:37 AM, mzhangw ***@***.***> wrote:
I am running RTs using master. Will submit PR later.
On Fri, Jun 14, 2019 at 10:25 AM Dom Heinzeller ***@***.***>
wrote:
> I assume we want a similar PR for the "master" branch afterwards, too?
> That one going into master will have to be held back and updated later,
> because the Vlab master updates are of higher priority. This PR here for
> the release can be reviewed and merged, if approved.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#271?email_source=notifications&email_token=AG7TW2SYIRQAP7OXVMLNGHTP2PA7HA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXI76Y#issuecomment-502173691>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AG7TW2W4SKTTJOFZSV3E2E3P2PA7HANCNFSM4HYJIXRA>
> .
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub <#271?email_source=notifications&email_token=AB5C2RJNVKA4GJG4I74ULP3P2PCK7A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ5EY#issuecomment-502177427>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB5C2RIOPIA2ORRIM7YWTODP2PCK7ANCNFSM4HYJIXRA>.
|
Collaborator
|
While you will have to update the code and rerun the RTs after the master
is updated from VLab, it is important that you complete the RTs you are
running now. The purpose is to be sure that this latest updates to the
documentation do not have any inadvertent effects on the simulations.
I took a look at the rendered html at
https://dtcenter.org/GMTB/v3.0/sci_doc/.
My recommendation is to accept this PR for the release. We will continue
augmenting and polishing the documentation after the release.
Ligia
On Fri, Jun 14, 2019 at 10:38 AM Dom Heinzeller <notifications@github.com>
wrote:
… Ok, you will have to update your code and rerun the regression tests after
the Vlab master updates went in, sorry.
> On Jun 14, 2019, at 10:37 AM, mzhangw ***@***.***> wrote:
>
> I am running RTs using master. Will submit PR later.
>
> On Fri, Jun 14, 2019 at 10:25 AM Dom Heinzeller <
***@***.***>
> wrote:
>
> > I assume we want a similar PR for the "master" branch afterwards, too?
> > That one going into master will have to be held back and updated later,
> > because the Vlab master updates are of higher priority. This PR here
for
> > the release can be reviewed and merged, if approved.
> >
> > —
> > You are receiving this because you authored the thread.
> > Reply to this email directly, view it on GitHub
> > <
#271?email_source=notifications&email_token=AG7TW2SYIRQAP7OXVMLNGHTP2PA7HA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXI76Y#issuecomment-502173691
>,
> > or mute the thread
> > <
https://github.com/notifications/unsubscribe-auth/AG7TW2W4SKTTJOFZSV3E2E3P2PA7HANCNFSM4HYJIXRA
>
> > .
> >
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub <
#271?email_source=notifications&email_token=AB5C2RJNVKA4GJG4I74ULP3P2PCK7A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ5EY#issuecomment-502177427>,
or mute the thread <
https://github.com/notifications/unsubscribe-auth/AB5C2RIOPIA2ORRIM7YWTODP2PCK7ANCNFSM4HYJIXRA
>.
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#271?email_source=notifications&email_token=AE7WQAS237FGXPA7CO4S26LP2PCOXA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ7NI#issuecomment-502177717>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE7WQATWN7HZIWR4JH37EF3P2PCOXANCNFSM4HYJIXRA>
.
|
Collaborator
|
Have these changes been tested to work with SCM?
… On Jun 14, 2019, at 10:43 AM, ligiabernardet ***@***.***> wrote:
While you will have to update the code and rerun the RTs after the master
is updated from VLab, it is important that you complete the RTs you are
running now. The purpose is to be sure that this latest updates to the
documentation do not have any inadvertent effects on the simulations.
I took a look at the rendered html at
https://dtcenter.org/GMTB/v3.0/sci_doc/.
My recommendation is to accept this PR for the release. We will continue
augmenting and polishing the documentation after the release.
Ligia
On Fri, Jun 14, 2019 at 10:38 AM Dom Heinzeller ***@***.***>
wrote:
> Ok, you will have to update your code and rerun the regression tests after
> the Vlab master updates went in, sorry.
>
> > On Jun 14, 2019, at 10:37 AM, mzhangw ***@***.***> wrote:
> >
> > I am running RTs using master. Will submit PR later.
> >
> > On Fri, Jun 14, 2019 at 10:25 AM Dom Heinzeller <
> ***@***.***>
> > wrote:
> >
> > > I assume we want a similar PR for the "master" branch afterwards, too?
> > > That one going into master will have to be held back and updated later,
> > > because the Vlab master updates are of higher priority. This PR here
> for
> > > the release can be reviewed and merged, if approved.
> > >
> > > —
> > > You are receiving this because you authored the thread.
> > > Reply to this email directly, view it on GitHub
> > > <
> #271?email_source=notifications&email_token=AG7TW2SYIRQAP7OXVMLNGHTP2PA7HA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXI76Y#issuecomment-502173691
> >,
> > > or mute the thread
> > > <
> https://github.com/notifications/unsubscribe-auth/AG7TW2W4SKTTJOFZSV3E2E3P2PA7HANCNFSM4HYJIXRA
> >
> > > .
> > >
> > —
> > You are receiving this because your review was requested.
> > Reply to this email directly, view it on GitHub <
> #271?email_source=notifications&email_token=AB5C2RJNVKA4GJG4I74ULP3P2PCK7A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ5EY#issuecomment-502177427>,
> or mute the thread <
> https://github.com/notifications/unsubscribe-auth/AB5C2RIOPIA2ORRIM7YWTODP2PCK7ANCNFSM4HYJIXRA
> >.
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#271?email_source=notifications&email_token=AE7WQAS237FGXPA7CO4S26LP2PCOXA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ7NI#issuecomment-502177717>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AE7WQATWN7HZIWR4JH37EF3P2PCOXANCNFSM4HYJIXRA>
> .
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub <#271?email_source=notifications&email_token=AB5C2RN34X45WF7QZUTT4OTP2PDE3A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXLENI#issuecomment-502182453>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB5C2RMAHODN33ZCUZM5L73P2PDE3ANCNFSM4HYJIXRA>.
|
Contributor
Author
|
Yes. It passed prebuild, compilation and runs with different scum suits.
The plots seems ‘reasonable’ curves to me.
On Fri, Jun 14, 2019 at 10:46 AM Dom Heinzeller <notifications@github.com>
wrote:
… Have these changes been tested to work with SCM?
> On Jun 14, 2019, at 10:43 AM, ligiabernardet ***@***.***>
wrote:
>
> While you will have to update the code and rerun the RTs after the master
> is updated from VLab, it is important that you complete the RTs you are
> running now. The purpose is to be sure that this latest updates to the
> documentation do not have any inadvertent effects on the simulations.
>
> I took a look at the rendered html at
> https://dtcenter.org/GMTB/v3.0/sci_doc/.
>
> My recommendation is to accept this PR for the release. We will continue
> augmenting and polishing the documentation after the release.
>
> Ligia
>
> On Fri, Jun 14, 2019 at 10:38 AM Dom Heinzeller <
***@***.***>
> wrote:
>
> > Ok, you will have to update your code and rerun the regression tests
after
> > the Vlab master updates went in, sorry.
> >
> > > On Jun 14, 2019, at 10:37 AM, mzhangw ***@***.***>
wrote:
> > >
> > > I am running RTs using master. Will submit PR later.
> > >
> > > On Fri, Jun 14, 2019 at 10:25 AM Dom Heinzeller <
> > ***@***.***>
> > > wrote:
> > >
> > > > I assume we want a similar PR for the "master" branch afterwards,
too?
> > > > That one going into master will have to be held back and updated
later,
> > > > because the Vlab master updates are of higher priority. This PR
here
> > for
> > > > the release can be reviewed and merged, if approved.
> > > >
> > > > —
> > > > You are receiving this because you authored the thread.
> > > > Reply to this email directly, view it on GitHub
> > > > <
> >
#271?email_source=notifications&email_token=AG7TW2SYIRQAP7OXVMLNGHTP2PA7HA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXI76Y#issuecomment-502173691
> > >,
> > > > or mute the thread
> > > > <
> >
https://github.com/notifications/unsubscribe-auth/AG7TW2W4SKTTJOFZSV3E2E3P2PA7HANCNFSM4HYJIXRA
> > >
> > > > .
> > > >
> > > —
> > > You are receiving this because your review was requested.
> > > Reply to this email directly, view it on GitHub <
> >
#271?email_source=notifications&email_token=AB5C2RJNVKA4GJG4I74ULP3P2PCK7A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ5EY#issuecomment-502177427
>,
> > or mute the thread <
> >
https://github.com/notifications/unsubscribe-auth/AB5C2RIOPIA2ORRIM7YWTODP2PCK7ANCNFSM4HYJIXRA
> > >.
> > >
> >
> > —
> > You are receiving this because you commented.
> > Reply to this email directly, view it on GitHub
> > <
#271?email_source=notifications&email_token=AE7WQAS237FGXPA7CO4S26LP2PCOXA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXJ7NI#issuecomment-502177717
>,
> > or mute the thread
> > <
https://github.com/notifications/unsubscribe-auth/AE7WQATWN7HZIWR4JH37EF3P2PCOXANCNFSM4HYJIXRA
>
> > .
> >
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub <
#271?email_source=notifications&email_token=AB5C2RN34X45WF7QZUTT4OTP2PDE3A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXLENI#issuecomment-502182453>,
or mute the thread <
https://github.com/notifications/unsubscribe-auth/AB5C2RMAHODN33ZCUZM5L73P2PDE3ANCNFSM4HYJIXRA
>.
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#271?email_source=notifications&email_token=AG7TW2XSYF6MLBGMVAZKMFDP2PDP5A5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXLL7Q#issuecomment-502183422>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG7TW2QPIGCY6AWFX2P56E3P2PDP5ANCNFSM4HYJIXRA>
.
|
climbfuji
approved these changes
Jun 14, 2019
Collaborator
climbfuji
left a comment
climbfuji
left a comment
There was a problem hiding this comment.
Approved based on feedback from @ligiabernardet and @mzhangw. I did not test or inspect the code changes myself. @grantfirl please feel free to jump in and merge.
climbfuji
requested changes
Jun 14, 2019
Collaborator
There was a problem hiding this comment.
Note - need to revert my approval, just saw that the PR does not only contain SciDoc changes, but also changes to CMakeLists.txt that should not go into the release branch. Please remove those commits for the PR to release v3.o.
Merging those commits in without removing the CMakeLists.txt changes will break gmtb-scm. Also, the SciDoc update should only contain Scidoc changes, nothing else.
Collaborator
|
I ran Man's branch with the SCM and did not run into any trouble on my machine. However, I agree with Dom that the CMakeLists.txt change was a change that went into the master branch but not v3.0, so this should be removed. I think the problem was that the branch started from master and not v3.0 when it was created. Man, you should be able to see which commits master is ahead of v3.0 and just revert those specific commits in your branch. As far as running doxygen, it works fine on my machine. One thing that I did notice is that there are 3 doxygen configuration files in ccpp-physics/physics/docs. I assumed that I should be using ccppv3_doxyfile, but it might be confusing to other users to have to choose. |
Contributor
Author
|
Thanks for the explanation and suggestion. modification is under way.
…On Fri, Jun 14, 2019 at 11:42 AM grantfirl ***@***.***> wrote:
I ran Man's branch with the SCM and did not run into any trouble on my
machine. However, I agree with Dom that the CMakeLists.txt change was a
change that went into the master branch but not v3.0, so this should be
removed. I think the problem was that the branch started from master and
not v3.0 when it was created. Man, you should be able to see which commits
master is ahead of v3.0 and just revert those specific commits in your
branch.
As far as running doxygen, it works fine on my machine. One thing that I
did notice is that there are 3 doxygen configuration files in
ccpp-physics/physics/docs. I assumed that I should be using
ccppv3_doxyfile, but it might be confusing to other users to have to choose.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#271?email_source=notifications&email_token=AG7TW2VSTXPSTSZG6VE3XF3P2PKAJA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXPX3I#issuecomment-502201325>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AG7TW2RJUD3CNPYQLBELJZLP2PKAJANCNFSM4HYJIXRA>
.
|
Collaborator
|
BTW, @mzhangw this is an impressive amount of work! |
Contributor
Author
|
I reverted CMakeLists.txt and deleted doxyfiles for previous version. |
Collaborator
|
I would have preferred to remove the commits that introduced the CMakeLists.txt changes rather than manually reverting and creating a new commit. But since this code is going to release 3.0, I leave it up to @grantfirl. |
Merged
Collaborator
|
@climbfuji @ligiabernardet @mzhangw I'm ready to approve either this PR or #273 into v3.0. @climbfuji #273 contains identical changes to this PR but I used |
Collaborator
|
From my review of the HTML rendition, the content is in good shape to
approve.
…On Fri, Jun 14, 2019 at 3:56 PM grantfirl ***@***.***> wrote:
@climbfuji <https://github.com/climbfuji> @ligiabernardet
<https://github.com/ligiabernardet> @mzhangw <https://github.com/mzhangw>
I'm ready to approve either this PR or #273
<#273> into v3.0. @climbfuji
<https://github.com/climbfuji> #273
<#273> contains identical
changes to this PR but I used git rebase to remove those 2 commits. Is
that what you had in mind?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#271?email_source=notifications&email_token=AE7WQAR6AD2SVHP67EC2Y7DP2QHZXA5CNFSM4HYJIXRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYGBSA#issuecomment-502292680>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE7WQAQFQPQ6MKOIFHR7HU3P2QHZXANCNFSM4HYJIXRA>
.
|
Collaborator
|
I just approved #273 in favor over this PR. |
hannahcbarnes
pushed a commit
to hannahcbarnes/ccpp-physics
that referenced
this pull request
Aug 3, 2022
…> wat, merra2 threading (NCAR#279) * changed .gitmodules to point to merra2 ccpp/physics * remove GFDL_atmos_cubed_sphere and ccpp-framework from .git module * remove IPD gfsphysics * Update .gitmodules and submodule pointer for ccpp-physics for code review and testing * Remove interstitial zorl composites * Update .gitmodules and submodule pointer fpor ccpp-physics for code review and testing * Remove or replace references to IPD in comments in atmos_model.F90 * Initialize Sfcprop%zorlx to clear_val instead of huge * Update submodule pointer for ccpp-physics * Rename Fortran variables and CCPP standard names / long names of surface composites from ocean to water * Rename Sfcprop%zorlw to Sfcprop%zorlwav * Rename Sfcprop%zorlo to Sfcprop%zorlw * update submodule pointer for ccpp-physics * Revert change to .gitmodules and update submodule pointer for ccpp-physics Co-authored-by: anning.cheng <anning.cheng@noaa.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here is the scidoc updates for CCPPV3 release