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

Matching to theory prediction #29

Merged
merged 90 commits into from
Aug 2, 2022
Merged

Matching to theory prediction #29

merged 90 commits into from
Aug 2, 2022

Conversation

giacomomagni
Copy link
Contributor

@giacomomagni giacomomagni commented Jul 13, 2022

This PR is for the implementation of the matching to theory predictions

@Radonirinaunimi Radonirinaunimi self-requested a review July 14, 2022 08:02
Copy link
Member

@Radonirinaunimi Radonirinaunimi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the convention TYPE_DATASETNAME_OBSTYPE_EXTRA (where EXTRA can be an additional specificity of the dataset) for consistency and easier manipulation.

PS: I might have missed some places where this appear in the suggestions below.

src/nnusf/data/matching_grids.py Outdated Show resolved Hide resolved
src/nnusf/theory/highq/runcards.py Outdated Show resolved Hide resolved
src/nnusf/data/loader.py Outdated Show resolved Hide resolved
src/nnusf/data/loader.py Outdated Show resolved Hide resolved
src/nnusf/data/loader.py Outdated Show resolved Hide resolved
src/nnusf/data/coefficients.py Outdated Show resolved Hide resolved
Copy link
Member

@Radonirinaunimi Radonirinaunimi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The below is just a matter of convention again.

For clarity, I'd also use destination_path instead of just dest and dataset_path instead of data_arg in the click definitions.

src/nnusf/cli/data.py Outdated Show resolved Hide resolved
@giacomomagni
Copy link
Contributor Author

We should use the convention TYPE_DATASETNAME_OBSTYPE_EXTRA (where EXTRA can be an additional specificity of the dataset) for consistency and easier manipulation.

PS: I might have missed some places where this appear in the suggestions below.

Have you tried that this is working?
I didn't append matching at the end, because there were some issue. But if you say this is the syntax I'm fine with it.

@Radonirinaunimi
Copy link
Member

We should use the convention TYPE_DATASETNAME_OBSTYPE_EXTRA (where EXTRA can be an additional specificity of the dataset) for consistency and easier manipulation.
PS: I might have missed some places where this appear in the suggestions below.

Have you tried that this is working? I didn't append matching at the end, because there were some issue. But if you say this is the syntax I'm fine with it.

I haven't tried this yet given that I expected that this should not change anything, but let me actually do the change locally.

@giacomomagni
Copy link
Contributor Author

I haven't tried this yet given that I expected that this should not change anything, but let me actually do the change locally.

Don't worry I should have fixed, thanks in any case.

@Radonirinaunimi
Copy link
Member

Merging this given that its main purpose (namely building the framework to generate and fit Yadism-like datasets) is achieved. Issues related to fit stability are/will be investigated in other PRs.

@Radonirinaunimi Radonirinaunimi merged commit 8561f0a into main Aug 2, 2022
@Radonirinaunimi Radonirinaunimi deleted the matching-grids branch August 2, 2022 14:16
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.

None yet

5 participants