-
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 all 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,39 +277,89 @@ 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: | ||
|
|
||
| # With shifts | ||
| if with_shift: | ||
| shifts = 0. | ||
| lcd_wc = loaded_commondata_with_cuts(commondata, cuts) | ||
| fail = False | ||
|
|
||
| # Determine theory shift | ||
| if not isinstance(result, DataResult): | ||
| err[mask] = result.std_error | ||
| theory_predictions = result.central_value | ||
| 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." | ||
| ) | ||
| fail = True | ||
|
|
||
| # Determine data uncertainty | ||
| else: | ||
| alpha = unco_unc(lcd_wc) | ||
| if alpha.all() == 0. or fail: | ||
|
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. queston, if fails, it should fail for everyone, right?
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. What do you mean?
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. In a plot we have data, theory1, theory2. Any failure should remove the shifts for everyone, right?
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. Ah! In that sense. So yes, if it fails for one theory, but not for another, then the plot must collapse to the unshifted version for everything. I hadn't thought about this case which, I think, is not encompassed by the current structure.
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. because right now, if I understand it correctly, if the data comes first the fail will never activate?
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. I think that now theory comes first. The way I understand your comment is the following: suppose you are comparing theory 1 and theory 2. Shifts are set to 0 initially. Then suppose that theory 1 fails because of the error in computing shifts. Then everything falls back to the unshifted version of the plot. Then suppose that theory 2 also fails for the same reason. No problem, everything falls back to the unshifted version of the plot. I understand that you're saying that there is an inconsistency when only one of the two theories fails, irrespective of the order. In the current implementation, the plot never falls back to the unshifted version, because some theories will be shifted and some will not, and the error will be the total error or only the uncorrelated part of the error depending on whether the failing theory is the last (or not). Is this what you are thinking of?
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. BTW, do you remember for which data set the computation of the shifts was failing?
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.
But more generally, the fact that it depends on the order and that is not guaranteed.
Yes, probably
No ^^U |
||
| err[mask] = result.std_error | ||
| with_shift = False | ||
| else: | ||
| err[mask] = alpha | ||
|
|
||
| cv[mask] = result.central_value - shifts | ||
|
|
||
| # 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 | ||
| err[mask] = result.std_error | ||
|
|
||
| cv, err = transform_result(cv, err, table.iloc[:, :nkinlabels], info) | ||
|
|
||
| """ | ||
| # Compute shifts due to the correlated part on the experimental ucnertainty | ||
| if with_shift and not isinstance(result, DataResult): | ||
| lcd_wc = loaded_commondata_with_cuts(commondata, cuts) | ||
| theory_predictions = result.central_value | ||
|
|
||
| # Check if the uncorrelated part of the uncertainty is zero or non-zero | ||
| #alpha = unco_unc(lcd_wc) | ||
| if alpha.all() == 0.: | ||
| shifts = 0. | ||
| err[mask] = result.std_error | ||
| with_shift = False | ||
| else: | ||
|
|
||
| # 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) | ||
| err[mask] = alpha | ||
| 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. | ||
| err[mask] = result.std_error | ||
| with_shift = False | ||
|
|
||
| cv[mask] = result.central_value - shifts | ||
|
|
||
| # No shifts | ||
| else: | ||
| cv[mask] = result.central_value | ||
| err[mask] = result.std_error | ||
|
|
||
| cv, err = transform_result(cv, err, table.iloc[:, :nkinlabels], info) | ||
| """ | ||
|
|
||
|
|
||
| # By doing tuple keys we avoid all possible name collisions | ||
| cvcol = ('cv', i) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.