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

Auto validate qc #167

Merged
merged 10 commits into from
Oct 5, 2018
Merged

Conversation

mattdturner
Copy link
Contributor

@mattdturner mattdturner commented Aug 9, 2018

Create new scripts to perform an auto-validation test for the QC script

  • 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

  • Is the documentation being updated with this PR? (Y/N) Y
    If not, does the documentation need to be updated separately at a later time? (Y/N) N
    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:

  • The process to run these scripts is:

cp configuration/scripts/tests/QC/gen_qc_cases.csh .   # from the root CICE directory
cp configuration/scripts/tests/QC/compare_qc_cases.csh .
./gen_qc_cases.csh -m <machine name> --acct <queue manager account>
# Wait for simulations to finish running
./compare_qc_cases.csh

Sample output from compare_qc_cases.csh:

===== Running QC tests and writing output to validate_qc.log =====
Running QC test on base and bfb directories.
Expected result: PASSED
Result: PASSED
-----------------------------------------------
Running QC test on base and non-bfb directories.
Expected result: PASSED
Result: PASSED
-----------------------------------------------
Running QC test on base and climate-changing directories.
Expected result: FAILED
Result: FAILED


QC Test has validated

turner added 3 commits August 8, 2018 16:47
….sh) generates the 4 cases necessary to validate the QC script. It then builds the executables and submits the tests to the queue. The 2nd script (compare_qc_cases.sh) then runs the QC script on the 3 combinations of tests to confirm whether or not the QC script is behaving properly
@mattdturner
Copy link
Contributor Author

This PR addresses part of #166 . It is NOT READY to be merged.

I would like some input on the method used to automate the tests. Once the scripts are finalized, I will update documentation to include information about using these scripts.

@apcraig
Copy link
Contributor

apcraig commented Aug 10, 2018

I think the implementation looks good. Just another thought though, would it make sense to add a -qc option to the cice.setup script that would invoke the qc script. I am not opposed to have a separate script to create the qc tests but it also seems a little redundant with what we have in cice.setup. The "-qc" option would not allow --case, --test, or --suite to be set, but some of the other options would be handy (maybe even --set?). The qc option could run a separate qc script once the inputs are read. We might even be able to leverage the "test suite waiting when reporting" feature to ultimately run the compare_qc_cases.csh at the end. In that way, we might be able to run the whole thing end to end with just a cice.setup call. Thoughts, concerns?

@mattdturner
Copy link
Contributor Author

I agree that it is a bit redundant. My only concern with adding the -qc option is that these scripts are only to validate the QC script. It might be a bit confusing to users to have a -qc option that doesn't actually perform the QC test.

My concern with leveraging the "test suite waiting when reporting" feature is that the QC simulations are long. I currently have it setup to run the 4 QC tests with the long namelist option. On Conrad, this results in an 8 hour walltime. The simulation actually takes about 1 hour, although I have left the option to run gx1. So the queue time and runtime combine to potentially take more than 24 hours. If we think its acceptable to expect the user to leave the terminal open for that long, I can setup the QC validation to all happen in a single script.

I still need to think of a more user-friendly solution to the current QC testing (not QC script validation) process.

@apcraig
Copy link
Contributor

apcraig commented Aug 10, 2018

OK, I agree. I thought this was to run the qc script. If this is just to test/validate the script, then I agree it belongs as a separate script under a subdirectory which probably will be run occasionally and only by a few of us. thanks.

@mattdturner
Copy link
Contributor Author

Ok. I'll update the documentation, and then this PR should be ready to be reviewed and merged.

@mattdturner
Copy link
Contributor Author

I updated the documentation to include information about validating the QC scripts. The updated documentation can be found at https://mattdturner-cice.readthedocs.io/en/latest/developer_guide/dg_scripts.html#code-compliance-script

Unless anybody has any comments or suggestions, I think this PR is ready to be merged. I'll include updates to the QC process in a later PR.

@apcraig
Copy link
Contributor

apcraig commented Aug 10, 2018

I am testing this myself on conrad now, should have feedback soon.

In the documentation, we need to make sure it's clear this is a test of the qc scripts, this is NOT the qc test. Looking at the documentation, it's in the right place, but I would just make this a little more explicit and clear in the first paragraph. I would make the first paragraph something like

The code compliance test validates non bit-for-bit model changes. The directory configuration/scripts/tests/QC contains scripts related to the compliance testing, and this process is described in detail in <Code Compliance Test (non bit-for-bit validation)>. This section will describe a set of scripts that test and validate the code compliance process. This should be done when the compliance test or compliance test scripts (ie. cice.t-test.py) are modified. Again, this section documents a validation process for the compliance scripts, it does not describe how to run the compliance test itself.

Two scripts have been created to automatically validate the code compliance script. These scripts are:

…the fact that the section describes the validation of the compliance script, not how to run the compliance test itself.
@mattdturner
Copy link
Contributor Author

Ok, I documented the documentation per your comments. The above readthedocs.io link should reflect the updates.

@apcraig
Copy link
Contributor

apcraig commented Aug 14, 2018

Thanks for updating the documentation. I am trying to run the test and having problems with the python script. I have tried loading netcdf and python modules and it still is aborting on conrad. I have also tried to run the pip commands documented in 3.3.4.3 of the documentation and pip does not exist on conrad as far as I can tell.

/p/home/apcraig/cice-consortium/cice.matt.vqc>pip install --user netCDF4
pip: Command not found.

./configuration/scripts/tests/QC/cice.t-test.py $base_histdir $bfb_histdir

Traceback (most recent call last):
File "./configuration/scripts/tests/QC/cice.t-test.py", line 13, in
import numpy as np
ImportError: No module named numpy

Traceback (most recent call last):
File "./configuration/scripts/tests/QC/cice.t-test.py", line 15, in
import netCDF4 as nc
ImportError: No module named netCDF4

With the CICE code, we provide the modules and env. That is not happening with this tool. We need some documentation in 4.5.7 for this.

Also, I have not run the QC test before, but I'm not sure I would be able to setup my environment on conrad so I could. How do we address this issue better both for the QC code and for the scripts that check the code compliance tool. Maybe it's just me, but still.....

The other thing is that it is not convenient to have to rerun the qc validation tool if something does fail. That requires manual deletion of the current cases. Something should be added to the gen_qc_cases.csh tool so it checks if the case already exists and if it does, it renames the old one to preserve it. In fact, there is some risk in that in terms of mixing things up. My preference might be for a --testid to be needed to the gen_qc_cases.csh and for a new directory to be created (like for test suites) where all the cases and the qc_logs directory will be added for that "suite". That makes is a lot easier to cleanly delete a prior test as well as preserve multiple attempts. Would that be possible?

@apcraig
Copy link
Contributor

apcraig commented Aug 31, 2018

@mattdturner, just want to followup. Any ideas on the problems I had with pip on conrad and other comments? It would be good to get this PR sorted out and closed. I will try again and see if I can figure it out.

@apcraig
Copy link
Contributor

apcraig commented Aug 31, 2018

OK, I got it to work for me. I had to do the following

load the appropriate python module
load the appropriate compilers
pip install --user numpy
pip install --user netCDF4 (which needs certain compilers to build)

Then it worked. So I would say there are two outstanding issues

  1. update the documentation in 4.5.7 to indicate what is needed in the environment to run the script. This should be similar to what's in 3.3.4.3.
  2. think about whether there is an easy way to support rerunning the test if there is a problem. right now, that involves manually cleaning cases. maybe that's ok, but it should be documented. but maybe we could do better using a --testid type argument or having a clean script. at the very least, gen_qc_cases.csh should check whether the cases already exist and then either abort or rename them or something. there is also a possible requirement that you want to keep both the old and new cases around for reference. when i try to run gen_gc_cases.csh when the cases already exist, the script fails ungracefully.

thanks!

@mattdturner
Copy link
Contributor Author

I finally got around to updating this branch. I modified gen_qc_cases.csh to gracefully exit if the case directory already exists. I also added the --testid option, which allows users to have multiple validation tests in the same directory.

The documentation has been updated to include the --testid option, and to provide a bit of information about the requirements (Python modules) for running the qc test.

Copy link
Contributor

@apcraig apcraig 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 the recent updates. I think this looks fine now.

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.

Since this is a test of a testing process, I think we should merge it so that it can be tested as part of our weekend testing.......

@eclare108213 eclare108213 merged commit 4701816 into CICE-Consortium:master Oct 5, 2018
@mattdturner mattdturner deleted the auto_validate_qc 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