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

Add new --qc option to cice.setup #214

Closed
wants to merge 3 commits into from

Conversation

mattdturner
Copy link
Contributor

Add new --qc option to cice.setup

  • Developer(s): Matt Turner

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial? bfb

  • Please include the link to test results or paste the summary block from the bottom of the testing output below.

  • Does this PR create or have dependencies on Icepack or any other models?

  • Is the documentation being updated with this PR? (Y/N) N
    If not, does the documentation need to be updated separately at a later time? (Y/N) Y

Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:

This PR creates a new run_qc.csh script to perform the QC test.
To run the test, ./cice.setup -m <machine> --qc
The --qc option accepts:

  • --testid
  • --mach
  • --acct
  • --queue
  • --grid
  • --env
  • --pes
  • --set

The script runs from the current working directory. It then does the following:

  • Clone the master repository into a new directory (./CICE_master.${testid}).
  • Move to the master directory and run a QC simulation
  • Back to the main directory (where ./cice.setup was run)
  • Run a QC simulation (test)
  • Wait for jobs to finish
  • Perform the QC test on the 2 directories

The master case will not use the user-provided --set arguments. The test case will use the user-provided --set arguments. This allows new configurations to be compared to the current default.

The implementation is a bit unusual, since I use cice.setup to run run_qc.csh, which then runs cice.setup. Its an odd cyclical dependency, but it works.

The documentation still needs to be updated once the scripting is finalized

Once this PR is finalized and merged, it should complete #166

turner added 3 commits October 19, 2018 15:22
…ed, cice.setup runs the new run_qc.csh script and then exits.
…he --qc flag is passed. The new script clones the master repository and runs a qc simulation. It then also runs a qc simulation for the current working directory, and runs the QC test script on both directories.
@apcraig
Copy link
Contributor

apcraig commented Oct 26, 2018

I think this is OK, and we can merge it. But I think it has limitations. The main one is that it always checks out the master and uses that as the base for comparison. While that will be the case often, it probably will not always be that way. For instance, I am going to run some qc tests where I run the same model version but with two different compilers to see if the qc passes. We might also use the qc test on the same code with two different namelist settings. There could also be cases where it's not the master to compare to, but some earlier version of the code. I also am not sure we need a separate script that has many redundancies with the cice.setup script. It seems that could be a maintenance issue. Also, we have the script that tests the qc script to make sure it's working. It would be nice if that actually use the --qc feature so it actually tests that too. Will that qc test script do that now?

Personally, what I think might be easier and possibly more useful would be to have a simple script that a user could edit that would just simply clone two sandboxes (or if they exist already, just use them), setup the qc test in each sandbox, build and run them, and then do the final compare.

Again, I do see some value in this implementation and am willing to commit it to the master, but wonder whether it is the best approach.

@mattdturner
Copy link
Contributor Author

I agree that the implementation has some limitations. However, it was the only way I could think of to have the entire process be handled in a single step. It was also the best approach that I could come up with that included a --qc option to cice.setup without requiring a large rewrite of cice.setup.

I could rewrite it to be a user-edited script that clones two sandboxes and runs the qc tests. If I understand correctly, you are suggesting to have the users edit the script to point to:

  • two CICE repositories (or local directories)
  • the set of options for each sandbox
  • the account to use for job submission
  • the compiler for each sandbox

I have no problem doing that, it just doesn't seem (to me) to be much easier than just manually running the QC tests (all you really have to do is add qc as an option for each test).

@apcraig
Copy link
Contributor

apcraig commented Oct 31, 2018

It's a good point whether it's worth having a script at all given how easy it is to run the qc tests manually. I think we also need to decide whether the current implementation is worth committing and maintaining given the limitations. Maybe we should get input from others about what would be useful. Maybe I'll update issue #166 and we can get input from others.

@apcraig
Copy link
Contributor

apcraig commented Nov 16, 2018

We decided to defer this, see #166. I am going to close this for now. We can reopen in the future if needed.

@apcraig apcraig closed this Nov 16, 2018
@mattdturner mattdturner deleted the issue166 branch June 24, 2019 14:52
@mattdturner mattdturner restored the issue166 branch June 24, 2019 14:52
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