-
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
Improving reading and recreating fit pseudodata #1328
Conversation
d2e5304
to
9d9ae85
Compare
now that the replica generation is fast, can we not just call the action on a set of replicas? Is there really that much speed up from MP now? In fact since the action is in validphys, can we not just call |
This got added in #1081 I think with the nnpdf/n3fit/src/n3fit/scripts/n3fit_exec.py Line 131 in 74d9183
|
in particular, calling the API inside of a validphys action seems really horrible, I don't think this should be an internal action, or even exist at all |
Oh true actually, let me see if the --parallel flag works |
70ae840
to
ccde1b2
Compare
just to say, the function which I mentioned, |
how long does this take now with the collect approach @siranipour ? |
Ah ok haha, I'll try and get another PR for that then because the tests will fail until we regenerate a new regression fit.
Unfortunately I was a bit pushed for time, so I didn't get to fully benchmark it, but it still took a non negligible O(few minutes). I'll take a proper look and also play with the parallel flag and see what happens. |
ba389c9
to
62c4521
Compare
Greetings from your nice fit 🤖 !
Check the report carefully, and please buy me a ☕ , or better, a GPU 😉! |
Any ideas why the regression tests fail? They pass locally for me... |
0df61c3
to
2c49d34
Compare
Ahh I generated the new baseline with theory 200 which is why the tests are failing. Will fix |
2c49d34
to
30280ea
Compare
30280ea
to
747f7a5
Compare
@scarlehoff this PR flies in tandem with #1333, in that it reproduces the pseudodata a posteriori which is in principle the ones that are saved by #1333. I would appreciate if you could take a look at this soon too. The basic idea is that I've added actions that work on a per replica basis. I.e replicate the pseudodata of |
Co-authored-by: Juan M. Cruz-Martinez <juacrumar@lairen.eu>
Now that python is used to construct the pseudodata, I've updated the pseudodata.py function that regenerates the pseudodata of a given fit. It's now much easier to do this in parallel because we don't use swig objects anymore.
Closes #1323
The tests will, however, fail for now until I regenerate a PDF that saves its python pseudodata for the regression test. @wilsonmr, I may be misremembering but did you at some point have a PR which saved the pseudodata of a fit?
EDIT: This has turned into a bit of a general improvements PR.
fitting::savepseudodata: True
nnpdf/validphys2/src/validphys/n3fit_data.py
Line 386 in b636ae0
I've now changed this so it works on an individual replica and then
collect
ed it overreplicas
(I did this because it was useful for the recreation of a training validation mask.