Skip to content

Validphys with pineappl rebased to master#1578

Merged
scarlehoff merged 57 commits into
masterfrom
validphys_with_pineappl_rebased
Aug 1, 2022
Merged

Validphys with pineappl rebased to master#1578
scarlehoff merged 57 commits into
masterfrom
validphys_with_pineappl_rebased

Conversation

@scarlehoff
Copy link
Copy Markdown
Member

I've decided to not push on top of #1529 after the rebase just in case

But there's nothing new with respect to #1529 I just want to have both active at the same time.

scarlehoff added 30 commits July 8, 2022 12:36
… the function preparing the n3fit input so that it can be reused: huge memory saving
@github-actions
Copy link
Copy Markdown

Greetings from your nice fit 🤖 !
I have good news for you, I just finished my tasks:

Check the report carefully, and please buy me a ☕ , or better, a GPU 😉!

Comment thread validphys2/src/validphys/convolution.py
Comment thread validphys2/src/validphys/core.py
Comment thread validphys2/src/validphys/core.py Outdated
Comment thread validphys2/src/validphys/coredata.py
Comment thread validphys2/src/validphys/coredata.py
Comment thread validphys2/src/validphys/n3fit_data.py Outdated
Comment thread validphys2/src/validphys/n3fit_data.py Outdated
Comment thread validphys2/src/validphys/pineparser.py Outdated
@scarlehoff
Copy link
Copy Markdown
Member Author

scarlehoff commented Jul 20, 2022

@andreab1997 I've addressed the comments. In order to ensure there are no other rebase problems hidden, could you run a theory-uncertainties fit and then a report with the previous one you ran in order to check the results are unchanged?

Edit: I've pushed the commits to the wrong branch >:(

@andreab1997
Copy link
Copy Markdown
Contributor

@andreab1997 I've addressed the comments. In order to ensure there are no other rebase problems hidden, could you run a theory-uncertainties fit and then a report with the previous one you ran in order to check the results are unchanged?

Edit: I've pushed the commits to the wrong branch >:(

Yes sure, I will run it in a moment

@andreab1997
Copy link
Copy Markdown
Contributor

I approved but I am still running the fit. The moment I have the report I will post it here @scarlehoff

@andreab1997
Copy link
Copy Markdown
Contributor

@andreab1997 I've addressed the comments. In order to ensure there are no other rebase problems hidden, could you run a theory-uncertainties fit and then a report with the previous one you ran in order to check the results are unchanged?

Edit: I've pushed the commits to the wrong branch >:(

The fit just finished. It seems that the files .fitinfo, .params, .preproc and .sumrules are not there. Is this wanted or an error? In both cases postfit can't run without that files @scarlehoff

@scarlehoff
Copy link
Copy Markdown
Member Author

This is not an error but postfit should run! Maybe you have an old installation of postfit?

@andreab1997
Copy link
Copy Markdown
Contributor

This is not an error but postfit should run! Maybe you have an old installation of postfit?

yes, it is possbile. Let me try

@andreab1997
Copy link
Copy Markdown
Contributor

Ok you were right. This is the link of the fit https://data.nnpdf.science/fits/220325-ab-nnpdf31like_wthcovmat_fullcovmat_test.tar.gz.
I am running now the comparison.

@andreab1997
Copy link
Copy Markdown
Contributor

Here is the comparison report.
https://vp.nnpdf.science/0-syr4ezTpqoYO8odcRoLA==
I believe that they are actually equivalent :) @scarlehoff

@scarlehoff
Copy link
Copy Markdown
Member Author

Looks great! Thanks!

@andreab1997
Copy link
Copy Markdown
Contributor

Looks great! Thanks!

I am also trying the new theories now. I you want I can post here the results before merging

@andreab1997
Copy link
Copy Markdown
Contributor

And if is not a problem, I would add the new dictionaries to


in this PR

@andreab1997
Copy link
Copy Markdown
Contributor

Just as a proof of concept these are the reports of the thcovmat plots with 3pt prescription for scheme-B and scheme C
https://vp.nnpdf.science/T-LvbshRQ3CskNCRgm0xXg==
https://vp.nnpdf.science/6RmobzooSsOGycj3-eGx_g==

@andreab1997
Copy link
Copy Markdown
Contributor

And just for reference I add here some other reports
https://vp.nnpdf.science/fbn88G-dRPqULc7rpKsgcg==
https://vp.nnpdf.science/S6brmMCPRjaCR_AAK4kJ4g==

@scarlehoff
Copy link
Copy Markdown
Member Author

Since this has been tested quit a bit by now, I'll merge it soon if no bugs are found

@scarlehoff
Copy link
Copy Markdown
Member Author

@andreab1997 are the changes in the two python files intentional? (they look like they are from black but one of them changes a lot of lines so just to be sure)

@andreab1997
Copy link
Copy Markdown
Contributor

@andreab1997 are the changes in the two python files intentional? (they look like they are from black but one of them changes a lot of lines so just to be sure)

Yes, they are from black: I was debugging stuffs so I decided to also run black. The only real change is in scalevariationstheoryids.yaml.

@scarlehoff
Copy link
Copy Markdown
Member Author

Ok, then. I will merge this after as soon as I manage to finish a fit (Galileo is a bit full so it might take a few days) since now there's you but also @giacomomagni who will need the stuff in this branch to proceed. At this point it should clearly be in master.

(and let's freeze it in its current state, otherwise once I manage to get the fit through I will have to restart it!)

@scarlehoff scarlehoff removed the run-fit-bot Starts fit bot from a PR. label Aug 1, 2022
@scarlehoff
Copy link
Copy Markdown
Member Author

Comparison between theory 400 (in its current form, so positivity / integrability is from theory 200) and NNPDF4.0 https://vp.nnpdf.science/be1bDitCQl2ndF4x2cd9pA==/

This together with the fit bot and the reports that @andreab1997 has already showed makes clear that there are no bugs (at least no bugs that would affect a fit or its comparison report).

So I'll merge this now. @andreab1997 @giacomomagni @goord when doing any fits that would require pineappl (or any features from this branch) please use master to ensure that we are testing master (and that bugs in master are caught!). This also closes #1541

C++ is only left in the closure test generation and common data loading (prioritizing that now from my side!)

@scarlehoff scarlehoff merged commit bbb6ced into master Aug 1, 2022
@scarlehoff scarlehoff deleted the validphys_with_pineappl_rebased branch August 1, 2022 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants