-
Notifications
You must be signed in to change notification settings - Fork 132
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
Auto validate qc #167
Conversation
…es that are being compared
….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
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. |
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? |
I agree that it is a bit redundant. My only concern with adding the 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 I still need to think of a more user-friendly solution to the current QC testing (not QC script validation) process. |
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. |
Ok. I'll update the documentation, and then this PR should be ready to be reviewed and merged. |
…on that details the QC script validation process.
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. |
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.
Ok, I documented the documentation per your comments. The above readthedocs.io link should reflect the updates. |
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 ./configuration/scripts/tests/QC/cice.t-test.py $base_histdir $bfb_histdir Traceback (most recent call last): Traceback (most recent call last): 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? |
@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. |
OK, I got it to work for me. I had to do the following load the appropriate python module Then it worked. So I would say there are two outstanding issues
thanks! |
…difications to check to ensure that each case is generated without error.
I finally got around to updating this branch. I modified The documentation has been updated to include the |
There was a problem hiding this 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.
There was a problem hiding this 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.......
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:
Sample output from compare_qc_cases.csh: