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

update suite.submit script and add new controls and cice.setup option… #395

Merged
merged 4 commits into from
Feb 14, 2020

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Jan 15, 2020

PR checklist

  • Short (1 sentence) summary of your PR:
    Update suite.submit script and add new features.
  • Developer(s):
    apcraig
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    Tested on izumi, defaults settings work fine and are backward compatible.
    Tested the suite.submit manually as well.
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

Added cice.setup options

  • --setup-only
  • --build-only
  • --run-interactive

Added ability to set env variables for use with suite.submit directory

  • SUITE_BUILD
  • SUITE_RUN
  • SUITE_SUBMIT

Removed suite.run script, this is now part of suite.submit script.

Also updated the documentation. This is an update of #357 based on the latest code and discussions in that PR.

@apcraig
Copy link
Contributor Author

apcraig commented Jan 15, 2020

I am closing #357.

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

Intuitively, I would interpret build-only as not including setup. Might it be more user-friendly to use --setup-build for that one?

@apcraig
Copy link
Contributor Author

apcraig commented Jan 17, 2020

I'm open about how to name the options. I think build-only implicitly means you have to create the cases, but understand these things can be tricky. Also, maybe we need 4 options. For clarity, how about

  • setup-only
  • setup-and-build-only
  • setup-and-build-and-run-interactive
  • setup-and-build-and-submit

That seems a little too wordy but is clear. Other proposals?

@eclare108213
Copy link
Contributor

How about

  • setup
  • setup-build
  • setup-build-run
  • setup-build-submit

@apcraig
Copy link
Contributor Author

apcraig commented Jan 17, 2020

How about

  • setup-only
  • setup-build
  • setup-build-run
  • setup-build-submit

@apcraig
Copy link
Contributor Author

apcraig commented Jan 17, 2020

I updated the options and committed. Can still make another change if needed.

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

looks fine to me

@phil-blain
Copy link
Member

I'm still looking at the tutorial activities and will have more time to test this next week...

@apcraig apcraig self-assigned this Jan 24, 2020
@apcraig
Copy link
Contributor Author

apcraig commented Jan 24, 2020

Just a quick update that we are waiting for @phil-blain to do a test/review. There is no rush or urgency, but that's why this PR is on hold for the moment.

Copy link
Member

@phil-blain phil-blain left a comment

Choose a reason for hiding this comment

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

This looks good. I noted a few things in the diff.

I also checked that with these changes the example I added some time ago to show how to use incremental compilation with test suites (Example 11) still makes sense; it does since the default behavior of suite.submit is build+submit.

Also, it would be admittedly easier to use the suite.submit script if any non-zero value for the SUITE_* variables would set them to true, and 0 would set them to false, such that:

SUITE_BUILD=1 SUITE_SUBMIT=0 ./suite.submit
SUITE_BUILD=0 ./suite.submit

would work. I realize this would make the argument checking more complicated so it's more of a "nice to have" I guess.


./cice.setup --suite quick_suite,base_suite --mach conrad --env cray,gnu --testid v01a --setup-only
cd testsuite.v01a
setenv SUITE_BUILD true
Copy link
Member

Choose a reason for hiding this comment

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

This would not work in a POSIX shell ... I'd at least prefix this example with "If you are using csh/tcsh", and then add a separate example for Bash, something like

# build the executables
SUITE_BUILD=true SUITE_SUBMIT=false ./suite.submit
# submit the test suite
SUITE_BUILD=false ./suite.submit
# run the test suite interactively
SUITE_BUILD=false SUITE_RUN=true SUITE_SUBMIT=false ./suite.submit

We do define the default values before, so I think it would be ok to just list the required values.

Please see :ref:`case_options` and :ref:`indtests` for more details about how these options are used.

As indicated above, **cice.setup** with ``--suite`` will create a directory called testsuite.[testid]. **cice.setup** also generates a script called **suite.submit** in that directory. **suite.submit** is the script that builds and submits the various test cases in the test suite. The *cice.setup** options ``--setup-only``, ``--setup-build``, and ``--setup-build-run`` modify how **suite.submit** is run by **cice.setup**. **suite.submit** can also be run manually, and the environment variables, SUITE_BUILD (builds the testcases), SUITE_RUN (runs the testcases interactively), and SUITE_SUBMIT (submit the testcases to run) control **suite.submit**. The default values for SUITE_BUILD, SUITE_RUN, and SUITE_SUBMIT are effectively true, false, and true. By defining other values for those environment variables, users can control the suite script. When using **suite.submit** manually, the string ``true`` (all lowercase) is the only string that will turn on a feature, and both SUITE_RUN and SUITE_SUBMIT cannot be true at the same time. But by leveraging the **cice.setup** command line arguments ``--setup-only``, ``--setup-build``, and ``--setup-build-run`` as well as the environment variables SUITE_BUILD, SUITE_RUN, and SUITE_SUBMIT, users can run **cice.setup** and **suite.submit** in various combinations to quickly setup, setup and build, submit, resubmit, run interactively, or rebuild and resubmit full testsuites quickly and easily. See below for an example.
Copy link
Member

Choose a reason for hiding this comment

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

I'd break up this paragraph for readability:

Suggested change
As indicated above, **cice.setup** with ``--suite`` will create a directory called testsuite.[testid]. **cice.setup** also generates a script called **suite.submit** in that directory. **suite.submit** is the script that builds and submits the various test cases in the test suite. The *cice.setup** options ``--setup-only``, ``--setup-build``, and ``--setup-build-run`` modify how **suite.submit** is run by **cice.setup**. **suite.submit** can also be run manually, and the environment variables, SUITE_BUILD (builds the testcases), SUITE_RUN (runs the testcases interactively), and SUITE_SUBMIT (submit the testcases to run) control **suite.submit**. The default values for SUITE_BUILD, SUITE_RUN, and SUITE_SUBMIT are effectively true, false, and true. By defining other values for those environment variables, users can control the suite script. When using **suite.submit** manually, the string ``true`` (all lowercase) is the only string that will turn on a feature, and both SUITE_RUN and SUITE_SUBMIT cannot be true at the same time. But by leveraging the **cice.setup** command line arguments ``--setup-only``, ``--setup-build``, and ``--setup-build-run`` as well as the environment variables SUITE_BUILD, SUITE_RUN, and SUITE_SUBMIT, users can run **cice.setup** and **suite.submit** in various combinations to quickly setup, setup and build, submit, resubmit, run interactively, or rebuild and resubmit full testsuites quickly and easily. See below for an example.
As indicated above, **cice.setup** with ``--suite`` will create a directory called testsuite.[testid]. **cice.setup** also generates a script called **suite.submit** in that directory. **suite.submit** is the script that builds and submits the various test cases in the test suite.
The *cice.setup** options ``--setup-only``, ``--setup-build``, and ``--setup-build-run`` modify how **suite.submit** is run by **cice.setup**. The **suite.submit** script can also be run manually, and the environment variables, SUITE_BUILD (build the testcases), SUITE_RUN (run the testcases interactively), and SUITE_SUBMIT (submit the testcases to run) control its behaviour. The default values of these variables are:
::
SUITE_BUILD = true
SUITE_RUN = false
SUITE_SUBMIT = true
By defining other values for those environment variables, users can control the suite script. When using **suite.submit** manually, the string ``true`` (all lowercase) is the only string that will turn on a feature, and both SUITE_RUN and SUITE_SUBMIT cannot be true at the same time.
By leveraging the **cice.setup** command line arguments ``--setup-only``, ``--setup-build``, and ``--setup-build-run`` as well as the environment variables SUITE_BUILD, SUITE_RUN, and SUITE_SUBMIT, users can run **cice.setup** and **suite.submit** in various combinations to quickly setup, setup and build, submit, resubmit, run interactively, or rebuild and resubmit full testsuites quickly and easily. See below for an example.

@@ -573,6 +587,24 @@ Test Suite Examples
there is still no absolute guarantee the tests will be completed in the correct
sequence.

13) **Test Suite generation then manual build followed by manual submission**
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
13) **Test Suite generation then manual build followed by manual submission**
13) **Test suite generation then manual build followed by manual submission**

@apcraig
Copy link
Contributor Author

apcraig commented Feb 11, 2020

I just made some commits to update the documentation as suggested by @phil-blain. I think this improves things and have confirmed the formatting in the RTD output.

With regard to the csh vs bash syntax, my preference is to avoid providing multiple examples for multiple shells if it's something pretty basic, so instead, I have added a quick comment about that. I think for the conda stuff, given the install is different in different shells, then it should be documented. For this, where we're just setting an env variable, I think we should be able to rely on the community to understand that difference.

I think we can probably merge before the weekend testing unless there are other comments or feedback.

@phil-blain
Copy link
Member

looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants