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

make_bcs clarification of options #517

Merged
merged 40 commits into from
Feb 10, 2022
Merged

Conversation

biljanaorescanin
Copy link
Contributor

@biljanaorescanin biljanaorescanin commented Jan 10, 2022

This PR needs to make clear separation of what user is choosing on screen for boundary conditions creation. We had recent issue where under development branch was used as suggested default. Hopefully this PR will help with that.

I've added to PR two small fixes one is mask we use for EASE grid. To be aligned with what archived EASE grid boundary conditions used in past. Other is format when CatchCN_CLM4.5 boundary conditions are created.

Since all of the changes are only triggered during run for boundary conditions creation package it is a zerodiff PR.

@gmao-rreichle @sanAkel let me know if either of you wants me to add or remove something.


UPDATED 9 Feb 2022 by @gmao-rreichle:

The PR now includes the following changes:

  • make_bcs (primary user interface)
    • fixed bugs in functionality, incl. better protection against user input errors
    • improved documentation
    • cleanup
  • mkCatchParam.F90
    • improved log statements
  • create_README.csh
    • corrected documentation, incl. 'format' statements
    • cleanup
  • mk_GEOSldasRestarts.F90
    • fixed bug in handling of CatchmentCNCLM45 restarts

The PR is trivially 0-diff for all current model and GEOSldas tests because it only touches code that is not exercised in any of the current tests. The land group verified that bcs generated after and just before the above modifications are 0-diff.
(Note that the archived bcs in /discover/nobackup/ltakacs/bcs were generated with a much older version and are not 0-diff against bcs generated with the current make_bcs package. The land group verified that the archived bcs are scientifically equivalent to bcs generated with the current make_bcs package.)

@sdrabenh @rdkoster @biljanaorescanin

@biljanaorescanin biljanaorescanin added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Jan 10, 2022
@gmao-rreichle
Copy link
Contributor

@biljanaorescanin,
Thanks for putting this together. Re. your comment:

I've added to PR two small fixes one is mask we use for EASE grid. To be aligned with what archived EASE grid boundary conditions used in past.

Are the bcs generated before and after this change 0-diff? Even though there isn't a nightly test for make_bcs, we still need to make sure and test that the make_bcs package is 0-diff.

@gmao-rreichle gmao-rreichle changed the title make_bcs clear optons make_bcs clarification of options Jan 10, 2022
@gmao-rreichle gmao-rreichle added the documentation Improvements or additions to documentation label Jan 10, 2022
sanAkel
sanAkel previously approved these changes Jan 11, 2022
Copy link
Collaborator

@sanAkel sanAkel left a comment

Choose a reason for hiding this comment

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

Thanks for copying me on your PR!

@gmao-rreichle gmao-rreichle dismissed stale reviews from sanAkel via a42ebcb January 11, 2022 14:24
@biljanaorescanin
Copy link
Contributor Author

biljanaorescanin commented Jan 11, 2022

I changed run setup. Is that what you had in mind @gmao-rreichle ? Now user runs from build directory and makes on screen choice for experiment dir.

@biljanaorescanin
Copy link
Contributor Author

@gmao-rreichle was this what you had in mind for help message? I left usage text the same just moved it to be on screen if added "-h" .

@biljanaorescanin
Copy link
Contributor Author

@gmao-rreichle I've added revision so we can address the potential for specifying more than one set of bcs. It will just add to directory name all resolutions separated by "_" . I've removed left over module sourcing.
I've changed one line in latlon option that I think it was wrong but we can talk about it after you take a look.

@gmao-rreichle
Copy link
Contributor

@biljanaorescanin:
I finished the changes that I had on my list by adding two commits:
3541a12 contains the nitty-gritty changes that fix several problems with bad user inputs in the interactive questions and the logic of processing the information. There are also some revisions to the help text.
d6179f9 changes the order of the interactive questions into something more logical and adds the BCs version to the default output path. I put these changes into a separate commit so the xxdiff of the changes in the first commit still makes some sense.

There are two things left to do:

  1. If you haven't already done so, please proofread the bcs README file generated by create_README.csh. We need to make sure that the README file correctly reflects how the bcs package works and what it generates.
  2. Test the branch to make sure everything still works as intended.

@gmao-rreichle gmao-rreichle added 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) and removed 0 diff The changes in this pull request have verified to be zero-diff with the target branch. labels Feb 9, 2022
@gmao-rreichle gmao-rreichle removed the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Feb 10, 2022
@gmao-rreichle gmao-rreichle marked this pull request as ready for review February 10, 2022 03:24
@gmao-rreichle gmao-rreichle requested a review from a team as a code owner February 10, 2022 03:24
@sdrabenh sdrabenh merged commit 0f8e625 into develop Feb 10, 2022
@sdrabenh sdrabenh deleted the feature/borescan_fix_make_bcs branch February 10, 2022 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants