-
Notifications
You must be signed in to change notification settings - Fork 14
Fix shifts in plot_fancy_dataspecs #2484
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
base: master
Are you sure you want to change the base?
Changes from 6 commits
c0226c0
0589797
8d9e405
ccd20fe
8b16ce6
9a5085d
b72dfca
85a1c02
007e926
34de111
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,9 +25,9 @@ | |||||||||||||||||
| from validphys.checks import check_not_using_pdferr | ||||||||||||||||||
| from validphys.commondata import loaded_commondata_with_cuts | ||||||||||||||||||
| from validphys.core import CutsPolicy, MCStats, cut_mask, load_commondata | ||||||||||||||||||
| from validphys.covmats import shifts_from_systematics | ||||||||||||||||||
| from validphys.covmats import shifts_from_systematics, unco_unc | ||||||||||||||||||
| from validphys.plotoptions.core import get_info, kitable, transform_result | ||||||||||||||||||
| from validphys.results import chi2_stat_labels, chi2_stats | ||||||||||||||||||
| from validphys.results import chi2_stat_labels, chi2_stats, DataResult | ||||||||||||||||||
| from validphys.sumrules import POL_LIMS | ||||||||||||||||||
| from validphys.utils import sane_groupby_iter, scale_from_grid, split_ranges | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -263,10 +263,6 @@ def _plot_fancy_impl( | |||||||||||||||||
| nkinlabels = len(table.columns) | ||||||||||||||||||
| ndata = len(table) | ||||||||||||||||||
|
|
||||||||||||||||||
| # Compute shifts due to the correlated part of the exp cov matrix | ||||||||||||||||||
| lcd_wc = loaded_commondata_with_cuts(commondata, cutlist[0]) | ||||||||||||||||||
| theory_predictions = results[1].central_value | ||||||||||||||||||
|
|
||||||||||||||||||
| # This is easier than cheking every time | ||||||||||||||||||
| if labellist is None: | ||||||||||||||||||
| labellist = [None] * len(results) | ||||||||||||||||||
|
|
@@ -281,36 +277,40 @@ def _plot_fancy_impl( | |||||||||||||||||
| # We modify the table, so we pass only the label columns | ||||||||||||||||||
| norm_cv, _ = transform_result(cv, err, table.iloc[:, :nkinlabels], info) | ||||||||||||||||||
|
|
||||||||||||||||||
| cvcols = [] | ||||||||||||||||||
|
|
||||||||||||||||||
| # Compute systematic shifts | ||||||||||||||||||
| # For unknown reasons, `shifts_from_systematics` may | ||||||||||||||||||
| # randomly fails. If a LinAlgError is raised, shifts are not included in | ||||||||||||||||||
| # the final plot. | ||||||||||||||||||
| if with_shift: | ||||||||||||||||||
| try: | ||||||||||||||||||
| shifts, alpha = shifts_from_systematics(lcd_wc, theory_predictions) | ||||||||||||||||||
| except np.linalg.LinAlgError: | ||||||||||||||||||
| log.warning( | ||||||||||||||||||
| "Error occurred in computing systematic shifts for " | ||||||||||||||||||
| f"{info.ds_metadata.name}. These will be disregarded in the plots." | ||||||||||||||||||
| ) | ||||||||||||||||||
| with_shift = False | ||||||||||||||||||
|
|
||||||||||||||||||
| cvcols = [] | ||||||||||||||||||
|
|
||||||||||||||||||
| for i, (result, cuts) in enumerate(zip(results, cutlist)): | ||||||||||||||||||
| # We modify the table, so we pass only the label columns | ||||||||||||||||||
|
|
||||||||||||||||||
| mask = cut_mask(cuts) | ||||||||||||||||||
| cv = np.full(ndata, np.nan) | ||||||||||||||||||
| err = np.full(ndata, np.nan) | ||||||||||||||||||
| # Shift the theory when with_shift option is True | ||||||||||||||||||
| if i == 1 and with_shift: | ||||||||||||||||||
|
|
||||||||||||||||||
| shifts = None | ||||||||||||||||||
| alpha = None | ||||||||||||||||||
| do_shift = with_shift | ||||||||||||||||||
|
|
||||||||||||||||||
| # Compute shifts due to the correlated part on the experiemntal ucnertainty | ||||||||||||||||||
| if do_shift: | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||||||||||||||||||
| lcd_wc = loaded_commondata_with_cuts(commondata, cuts) | ||||||||||||||||||
| theory_predictions = result.central_value | ||||||||||||||||||
|
|
||||||||||||||||||
| # For unknown reasons, `shifts_from_systematics` may randomly fail. | ||||||||||||||||||
| # If a LinAlgError is raised, shifts are not included in the final plot. | ||||||||||||||||||
| try: | ||||||||||||||||||
| shifts = shifts_from_systematics(lcd_wc, theory_predictions) | ||||||||||||||||||
| except np.linalg.LinAlgError: | ||||||||||||||||||
| log.warning( | ||||||||||||||||||
| "Error occurred in computing systematic shifts for " | ||||||||||||||||||
| f"{info.ds_metadata.name}. These will be disregarded in the plots." | ||||||||||||||||||
| ) | ||||||||||||||||||
| shifts = 0. | ||||||||||||||||||
|
|
||||||||||||||||||
| cv[mask] = result.central_value - shifts | ||||||||||||||||||
| err[mask] = unco_unc(lcd_wc) | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the check for data/theory is missing with (but maybe then
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After some thoughts, I think that here, if try fails, we just don't want to apply the shifts, and we want to fall into the unshifted case by default (which is not the case now). I am thinking that perhaps we should fall in this case also when alpha is zero (that is when there's no uncorrelated part of the uncertainty because e.g. the experimentalists provide us only with a covariance matrix). Right now we just set the uncertainty displayed in data/theory plots to zero and do not shift the data (and say in the title that the data is shifted).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the way of making sure there is no shift applied is to do so that it is equivalent to not applying the shift, no?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not exactly, because I fear that the label printed on the plot will still be "shifted" because try is in the with_shift if. I'm checking right now.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simple fix, turn with_shift to False. |
||||||||||||||||||
|
|
||||||||||||||||||
| # No shifts | ||||||||||||||||||
| else: | ||||||||||||||||||
| cv[mask] = result.central_value | ||||||||||||||||||
| # Retain only the uncorrelated part of the error if shifting the data | ||||||||||||||||||
| if i == 0 and with_shift: | ||||||||||||||||||
| err[mask] = alpha | ||||||||||||||||||
| else: | ||||||||||||||||||
| err[mask] = result.std_error | ||||||||||||||||||
|
|
||||||||||||||||||
| cv, err = transform_result(cv, err, table.iloc[:, :nkinlabels], info) | ||||||||||||||||||
|
|
||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.