Add checks #31
Add checks #31
Conversation
Codecov Report
@@ Coverage Diff @@
## master #31 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 14 14
Lines 1297 1299 +2
======================================
- Misses 1297 1299 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@scarlehoff yes, that sounds interesting at this point. Unfortunately I don't have students now, but for sure we can consider as a master topic. |
|
I quite like how the pycompressor works, quite streamlined :) I've made some changes which are just style since I was reading through the code I thought it was silly to write comments about them since they are all very small changes. Some comments below: pycompressor/src/pycompressor/compressor.py Line 161 in 5ea7f0d Here the minimization is deprecated, why do you want to keep it? Wouldn't it be better to remove altogether (since you don't have to worry about backwards compatibility yet!) so you can remove all code pycompressor/src/pycompressor/pdfgrid.py Line 18 in 5ea7f0d Are you always using the same grid? I'm worried that then a change in the grid used by NNPDF or LHAPDF at some point could break the code. Maybe it would be good to have some check to ensure that the grid is the same (maybe an error for us in NNPDF to have the grid sort of fixed instead of being something generated by vp...) pycompressor/src/pycompressor/utils.py Line 36 in 5ea7f0d It makes me nervous in general to see Edit: please make sure I haven't broken anything with this changes before accepting the changes! |
Thanks for having a quick look 👍 Every comments are always useful :-)
Here, the CMA is deprecated in the sense that it is unable to perform an adiabatic minimization as the GA. It still can perform the a minimization but maybe not as efficient when it comes to compress from the enhanced (I haven't tested this much).
I think the gird here is pretty useless and could be in any form. The only part it matters is when generating the PDF grids that will be estimated (it does not go into the final output). Once the compression is done and the list of selected replicas is generated, what the compressor does is just to copy the exact file from the LHAPDF and rename the replica index.
Thanks a lot for this suggestion, I will do so.
|
src/pycompressor/compressing.py
Outdated
| members = pdfsetting["pdf"].load().GetMembers() | ||
| if members < compressed: | ||
| if not gans["enhance"] and not pdfsetting["existing_enhanced"]: | ||
| raise CheckError(f"Cannot get {requested_replicas} replicas from {members} members if enhancing is not active") | ||
| raise CheckError( | ||
| f" Cannot get {compressed} replicas from" | ||
| f" {members} members if enhancing is not active.") |
There was a problem hiding this comment.
I just fixed the GetMembers part as it was giving error. I also think that here what we would like to check is if the size of the compressed is smaller thatn the size of the prior, and then asked for enhancement if it is greater. I have also changed that.
|
Oups, I forgot to change the |
Co-authored-by: Juacrumar <juacrumar@lairen.eu>
Better handling of packaging and versioning.
Added a check so that the program stop before starting if it finds a problem in the runcard. I'll might add a few more.
I'm thinking about the pdfflow thing, but I think it would be need to implement it in validphys instead because all the other stuff is the lhapdf-data which we don't really have in pdfflow (@scarrazza, should we implement the LHAPDF-data stuff into pdfflow? Basically getting track of the pdfs in the server and the computer... do you have maybe a master student who wants to do network services going forward :P?)