-
Notifications
You must be signed in to change notification settings - Fork 1
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 theory order local per dataset #99
Conversation
@jacoterh Correct me if wrong, this will be fully compatible with current tables as long as we specify "NLO" or "LO" for each dataset, right? |
Yes, that's right, the only thing that mattes is that the order specified for each dataset also appears in the theory json. This can be any key as you point out, but PR#25 on smefit_database actually updates the keys in the jsons to our new convention (i.e. NLO_QCD, NLO_EW, etc) |
@jacoterh, is there any default behavior for the case in which one forgets to specify the order for a dataset? That could be useful in case one reuses an old runcard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor requests regarding default and limit behaviors.
Should we also update the documentation within this same PR before forgetting about this change?
Should be ready to go. Updated behaviour
where the predictions of |
Updated syntax (no mixed types anymore)
This syntax is future proof as it supports naturally additional properties we might want to specify per dataset, i.e. cuts. |
So now you made it a list of dictionaries, right? it's more similar to the NNPDF format. I am in favour of this but I thought we wanted to keep the functioning of the old runcards? |
That's right, it's a list of dictionaries, so it'd be equivalent to doing
This breaks compatibility with the old runcards, but for this we can provide a short script to convert them. In any case it's a small difference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one comment on the order datasets are looped through but for the rest it looks fine.
|
||
_logger.info(f"Applying cutoff scale: {cutoff_scale} GeV.") | ||
for sset in np.unique(datasets): | ||
for sset in datasets: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and in similar loops (the one in load_rge_mat for example) datasets are looped without being put in alphabetical order now? I am a bit worried about having mismatches, are we guaranteed that things are ordered correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your concern, but datasets is a list which has a well defined ordering, so there's no need to sort anything (as opposed to dictionaries). But it was a list also before, and yet it got sorted. Perhaps @giacomomagni can comment if/why this was necessary at the time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, so the order is always the one put in the card basically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the unique was there just to be sure a dataset is not loaded twice, but yes from the DataTuple
you can always infer the order.
use_quad, | ||
use_theory_covmat, | ||
use_t0, | ||
use_multiplicative_prescription, | ||
default_order="LO", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need to have a default "LO" ? Do you need to load a dataset without any theory being loaded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just the default theory order that gets loaded if one doesn't specify it for a particular dataset. Of course, the default can be set to NLO as well, but not all predictions are available at NLO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, but sorry to be picky why do you want to have a default ?
I mean if you load, you need to specify an order
|
||
_logger.info(f"Applying cutoff scale: {cutoff_scale} GeV.") | ||
for sset in np.unique(datasets): | ||
for sset in datasets: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the unique was there just to be sure a dataset is not loaded twice, but yes from the DataTuple
you can always infer the order.
Co-authored-by: Giacomo Magni <[email protected]>
This PR removes the
pQCD
order key in the runcard and replaces it by locally specifying it per dataset, e.g:The reason we need this feature is because we now have different higher order corrections that we like to distinguish:
NLO_QCD, NLO_EW, NLO_QCD_EW
. The theory files need updating accordingly, which is done in PR#25 on smefit_database.