search

NNPDF / reportengine

A framework for declarative data analysis
https://data.nnpdf.science/validphys-docs/guide.html
GNU General Public License v2.0
1 stars 2 forks source link

Add --dry option #46

Closed scarlehoff closed 3 years ago

scarlehoff commented 3 years ago

So that the checks are performed but the actions not run or resources downloaded (see https://github.com/NNPDF/nnpdf/issues/1184#issuecomment-811136451)

RoyStegeman commented 3 years ago

Not downloading the resources would make any dry run that tests downloadable resources fail, checks such as check_pdf_is_montecarlo. Did you have any way in mind to deal with this? One could of course make the dry run fail if such a resource has not yet been downloaded, but allow the dry run to use a resource if it has been downloaded earlier.

Alternatively I guess the dry run could 'succeed' if the resource has not been downloaded, but is also not used by any check related to a given action. Personally, I dislike this option since it can make things a bit unpredictable and it would not catch typos in the name of the resources (or we would have to allow vp to check if the requested resource can be downloaded..).

If we want to implement a dry run, I think I would be in favour of downloading the resources as part of a dry run. That way, if dry run has executed successfully we can safely do the actual run.

Since typing this may already have taken more lines than implementing it like that (as it's basically just removing the output environment and exiting reportengine between the checks and the execution of the actions), maybe you had other ideas? What specifically would you like to see implemented in reportengine and what in validphys?

scarlehoff commented 3 years ago

I had no specific idea in mind but I would favour the dry run to fail if it cannot run as it is (with the right msg). That would include not having the right resources locally.

RoyStegeman commented 3 years ago

Well the loading of resources is implemented at the level of validphys, reportengine isn't aware of the existence of FallbackLoader and parents. So I think in reportengine we could do something like: run checks -> remove output folder -> exit, but I think that what you have in mind would need to be implemented in validphys.

scarlehoff commented 3 years ago

With --dry, reportengine knows whether to run after the checks are done. The decision of what constitutes a check is on its children shoulders, of course. In this case vp should know that a --dry run is underway and skip some steps (like the downloads).

That said I won't oppose any implementation of this, but I'd prefer the driest of dry runs because anything else can be achieved pressing Ctrl+C when you see checks passed.

RoyStegeman commented 3 years ago

That said I won't oppose any implementation of this, but I'd prefer the driest of dry runs because anything else can be achieved pressing Ctrl+C when you see checks passed.

If you want I can add something like that. However, if you feel this way, what is the additional value of adding --dry vs --no-net followed by Ctrl+C? We can have --dry also delete the output folder, but that's about it.

scarlehoff commented 3 years ago

You can automatize it (for instance if editing cards in bulk). Also, Ctrl+C is ugly, even having the "downloadable version" would be better. I was just stating why I would prefer not to have the downloads start.