-
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
Changes from 13 commits
c0226c0
0589797
8d9e405
ccd20fe
8b16ce6
9a5085d
b72dfca
85a1c02
007e926
34de111
dcfc0c6
5d6c7b0
f82f989
7d93eba
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]) | ||
|
enocera marked this conversation as resolved.
|
||
| 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: | ||
| cv[mask] = result.central_value - shifts | ||
|
|
||
| # w/ shifts | ||
| if with_shift: | ||
|
scarlehoff marked this conversation as resolved.
|
||
| shifts = 0. | ||
| lcd_wc = loaded_commondata_with_cuts(commondata, cuts) | ||
| # Determine data uncertainty | ||
| if isinstance(result, DataResult): | ||
| cv[mask] = result.central_value | ||
| alpha = unco_unc(lcd_wc) | ||
| if alpha.all() == 0.: | ||
| err[mask] = result.std_error | ||
| with_shift = False | ||
| else: | ||
| err[mask] = alpha | ||
| # Determine shift | ||
| else: | ||
| if with_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. This second if will always be true, because if the previous sets it to False then the next run of the loop won't get here (since the first if will not pass). So I think you can remove the if/else. |
||
| theory_predictions = result.central_value | ||
| shifts = shifts_from_systematics(lcd_wc, theory_predictions) | ||
| cv[mask] = result.central_value - shifts | ||
| else: | ||
| cv[mask] = result.central_value | ||
| err[mask] = result.std_error | ||
|
|
||
| # w/o 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) | ||
|
|
||
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.
Ah, never mind, you are already doing that.
Then yes, simply remove the second if (the
elsepath can never be reached) and we are good to go.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.
Done, thanks!