-
Notifications
You must be signed in to change notification settings - Fork 6
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 regression tests for theory matrices #117
Comments
@Zaharid, could you please remind me how to go about doing this? In particular, which file do these regression tests go in? |
@voisey Have a look at: https://github.com/NNPDF/nnpdf/blob/master/validphys2/src/validphys/tests/test_regressions.py The idea is that you define a function starting with The function should return a pandas frame. The test works as follows: If no saved table is found in the designed path, if will be written to the path and the test will fail. if it exists, the computed version will be compared with the loaded version and the test will fail if the results are (too) different. In this way we will see it if some change in the code unintentionally changes the result, as the computed version will no longer match the stored version. |
Thanks for this but I'm still a little unclear on how this works exactly. Is the idea that in this function I use the theory_covmat action from theorycovariance.py to return a theory_covmat dataframe, so that we can test future covmats against this? |
@voisey sorry, this is another thing that is more folklore than documented. The tests are run by a program called pytest (that you can install with conda). You can see how we run it here: nnpdf/conda-recipe/run_test.sh Line 7 in 4efae63
if you run
you can be more selective with e.g. This causes, that among other things, the functions that start with Going back to test_regressions, yes, you need to execute manually the actions that reportengine performs in order to get to a covmat, which is itself a dataframe. |
Thanks for the info. I'll play around with this. It seems hypothesis is a requirement for pytest but it's not installed as a dependency when one does 'conda install pytest'. Should I open an issue for this? |
No, thst is fine. It's a requirement for the nnpdf tests, as specified by
our recipe.
…On Fri, 5 Oct 2018, 16:36 Cameron Voisey, ***@***.***> wrote:
Thanks for the info. I'll play around with this. It seems hypothesis is a
requirement for pytest but it's not installed as a dependency when one does
'conda install pytest'. Should I open an issue for this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#117 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFabUk3mdjRp-WvGM2GGPKyqaB9mUYsIks5uh3x5gaJpZM4SfCwh>
.
|
Ok, good. So far I have something like this:
Is this going at all in the right direction? Am I correct in using convolution_results? I need to provide theory_covmat with theoryids_experiments_central_values rather than what I currently call 'th' but I'm not sure how to do this. |
The start looks good. Roughly you can replace |
There were already some tests and the rest was done in #1899 |
It would be good to have regression tests on things like the 3 point covariance matrix, to ensure that further changes do not screw it up.
This depends on #113 to a large extend.
The text was updated successfully, but these errors were encountered: