Skip to content
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

Making n3fit use python pseudodata #1268

Merged
merged 5 commits into from
Jun 24, 2021
Merged

Making n3fit use python pseudodata #1268

merged 5 commits into from
Jun 24, 2021

Conversation

siranipour
Copy link
Contributor

No description provided.

@siranipour siranipour added the run-fit-bot Starts fit bot from a PR. label Jun 4, 2021
@wilsonmr
Copy link
Contributor

wilsonmr commented Jun 4, 2021

Nice!

We also want to be able to make closure data so I think the make_replica function will want to be able to take some different central values and generate data from there. I half wonder if one could add a method to load the filtered commondata to the CommonData class and then just explicit node loaded_commondata_with_cuts to either load the closure data or the regular commondata.. It's a smaller change but further ingrains the closure data system I know @Zaharid hates. The next thing is at some point we would also want the closure central values (or level 1 data) to be generated with the new code..

@wilsonmr
Copy link
Contributor

wilsonmr commented Jun 4, 2021

Actually wait, I think because the commondataparser takes a CommonDataSpec which, when use_fitcommondata=True, sets the data path to the filtered data, this might already do the right thing!

EDIT: opened an issue where I will test this #1269

@siranipour
Copy link
Contributor Author

Do we still want this seed logic:

base_mcseed = int(hashlib.sha256(str(data).encode()).hexdigest(), 16) % 10 ** 8

@Zaharid
Copy link
Contributor

Zaharid commented Jun 4, 2021

@siranipour probably we can do something simpler.

Do we still want this seed logic:

base_mcseed = int(hashlib.sha256(str(data).encode()).hexdigest(), 16) % 10 ** 8

@scarlehoff
Copy link
Member

scarlehoff commented Jun 7, 2021

The generate_data_replica function should be removed right?

Also, any changes to the seed logic I would prefer it if they are done in separate PRs since they will also need to change all regression tests + fit bot, etc

@siranipour
Copy link
Contributor Author

The generate_data_replica function should be removed right?

Yeah it can, I had just left it for now in case we wanted to use the old behaviour at any point

@siranipour
Copy link
Contributor Author

The generate_data_replica function should be removed right?

Also, any changes to the seed logic I would prefer it if they are done in separate PRs since they will also need to change all regression tests + fit bot, etc

I'm beginning to think the changes you mention here need to be done in this PR since the pseudodata generation does not replicate precisely the C++ pseudodata. I believe this is why the linux tests are failing currently

@scarlehoff
Copy link
Member

Oh. Didn't realise that. In that case yes, do the changes here. Leave just the bot for another PR (or for the very last commit) so that we have the report of this PR vs master.

@siranipour
Copy link
Contributor Author

@scarlehoff is there any neat way of generating the quickcard_{replica}.json regression files (e.g in validphys it generates them automagically when you first run pytest) or do I need to create them manually?

@scarlehoff
Copy link
Member

Nop, you have to do it manually.

@siranipour siranipour force-pushed the n3fit_pseudodata branch 2 times, most recently from d4b10e8 to 1790552 Compare June 9, 2021 15:36
@siranipour siranipour added run-fit-bot Starts fit bot from a PR. and removed run-fit-bot Starts fit bot from a PR. labels Jun 11, 2021
@github-actions
Copy link

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 😉!

@siranipour
Copy link
Contributor Author

@scarlehoff the bot has been fixed in 0b21f84 so this can be merged if you're happy

@scarlehoff scarlehoff merged commit 1def506 into master Jun 24, 2021
@scarlehoff scarlehoff deleted the n3fit_pseudodata branch June 24, 2021 11:57
@Zaharid Zaharid added the enhancement New feature or request label Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
destroyingc++ enhancement New feature or request run-fit-bot Starts fit bot from a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants