Skip to content

Miscelaneous bugfixes (matplotlib & futuretests) #1809

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.

Sign up for GitHub

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

Merged
scarlehoff merged 7 commits into from
Oct 5, 2023
Merged

Miscelaneous bugfixes (matplotlib & futuretests) #1809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
09d83e9
use get_next_color instead of prop_cycler
scarlehoff Oct 3, 2023
bc08092
fix check for montecarlo/symmhessian
scarlehoff Oct 3, 2023
e4d1f1e
pin matplotlib < 3.8
scarlehoff Oct 3, 2023
1c69689
update example
scarlehoff Oct 3, 2023
f68a74b
update the jupyter example to use the recommended resources and lates…
scarlehoff Oct 4, 2023
6c936f7
Update conda-recipe/meta.yaml
scarlehoff Oct 4, 2023
154a6dd
Update validphys2/src/validphys/checks.py
scarlehoff Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion conda-recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ requirements:
- pkg-config
- python # requirements for validphys
- reportengine >=0.30.25 # see https://github.com/NNPDF/reportengine
- matplotlib >=3.3.0
- matplotlib >=3.3.0,<3.8 # see https://github.com/NNPDF/nnpdf/pull/1809
- blessings >=1.7
- scipy >=0.19.1
- pandas <2 # In principle the code runs with pandas 2 but the results (plots, tables, formats) are untested
Expand Down
59 changes: 34 additions & 25 deletions validphys2/examples/API_example.ipynb
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions validphys2/src/validphys/checks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ def check_pdf_is_montecarlo(ns, **kwargs):


@make_argcheck
Copy link
Copy Markdown
Member

@comane comane Oct 4, 2023

Choose a reason for hiding this comment

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

Suggested change
@make_argcheck

def check_pdf_is_montecarlo_or_symmhessian(ns, **kwargs):
pdf = ns['pdf']
def check_pdf_is_montecarlo_or_symmhessian(pdf, **kwargs):
etype = pdf.error_type
Comment thread
Radonirinaunimi marked this conversation as resolved.
Comment on lines +47 to 48
Copy link
Copy Markdown
Member

@comane comane Oct 4, 2023

Choose a reason for hiding this comment

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

Suggested change
def check_pdf_is_montecarlo_or_symmhessian(pdf, **kwargs):
etype = pdf.error_type
@make_check
def check_pdf_is_montecarlo_or_symmhessian(ns, **kwargs):
pdf = ns['pdf']
etype = pdf.error_type

Copy link
Copy Markdown
Member Author

@scarlehoff scarlehoff Oct 4, 2023

Choose a reason for hiding this comment

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

In reportengine it seems make_argcheck is preferred https://github.com/NNPDF/reportengine/blob/79eec2e33c5aab3998fd58e9c3cf84bd2c2696e7/src/reportengine/checks.py#L10

why should we use make_check and ns instead? It also a bit clearer with arcgcheck and pdf since that check is only active for PDFs in any case.

Copy link
Copy Markdown
Member

@comane comane Oct 4, 2023

Choose a reason for hiding this comment

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

Sure, I am ok with that.

Copy link
Copy Markdown
Member Author

@scarlehoff scarlehoff Oct 4, 2023

Choose a reason for hiding this comment

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

But what's the reason for using one over the other?

Copy link
Copy Markdown
Member

@comane comane Oct 4, 2023

Choose a reason for hiding this comment

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

Not sure tbh, I was suggesting it because the other functions like check_pdf_is_montecarlo make use of it and hence it would be more uniform, but I changed my mind.

I think you are right though that make_check should not be used anymore since it is weird that one can access the namespace ns from validphys.

I am thinking that we could change the others to be make_argcheck as well at this point.

check(
etype in {'replicas', 'symhessian'},
etype in {'replicas', 'symmhessian'},
f"Error type of PDF {pdf} must be either 'replicas' or 'symmhessian' and not {etype}",
)

Expand Down
4 changes: 1 addition & 3 deletions validphys2/src/validphys/deltachi2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,7 @@ def draw(self, pdf, grid, flstate):

errorstd = stats.std_error()
# color cycle iterable
pcycler = ax._get_lines.prop_cycler
next_prop = next(pcycler)
color = next_prop["color"]
color = ax._get_lines.get_next_color()
Comment thread
scarlehoff marked this conversation as resolved.
xgrid = grid.xgrid
# the division by 2 is equivalent to considering the complete 1-sigma band (2 * error_std)
error68 = (error68up - error68down) / 2.0
Expand Down
13 changes: 4 additions & 9 deletions validphys2/src/validphys/pdfplots.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ def _warn_any_pdf_not_montecarlo(pdfs):
class ReplicaPDFPlotter(PDFPlotter):
def draw(self, pdf, grid, flstate):
ax = flstate.ax
next_prop = next(ax._get_lines.prop_cycler)
color = next_prop['color']
color = ax._get_lines.get_next_color()
flavour_grid = grid.select_flavour(flstate.flindex)
stats = flavour_grid.grid_values
gv = stats.data
Expand Down Expand Up @@ -345,9 +344,7 @@ def draw(self, pdf, grid, flstate):

ax = flstate.ax
flindex = flstate.flindex
pcycler = ax._get_lines.prop_cycler
next_prop = next(pcycler)
color = next_prop['color']
color = ax._get_lines.get_next_color()

# The grid for the distance is (1, flavours, points)
# take only the flavour we are interested in
Expand Down Expand Up @@ -445,7 +442,6 @@ def draw(self, pdf, grid, flstate):
# Take only the flavours we are interested in
stats = grid.select_flavour(flstate.flindex).grid_values

next_prop = next(ax._get_lines.prop_cycler)
cv = stats.central_value()
xgrid = grid.xgrid
# Ignore spurious normalization warnings
Expand All @@ -455,7 +451,7 @@ def draw(self, pdf, grid, flstate):

# http://stackoverflow.com/questions/5195466/matplotlib-does-not-display-hatching-when-rendering-to-pdf
hatch = next(hatchit)
color = next_prop['color']
color = ax._get_lines.get_next_color()
(cvline,) = ax.plot(xgrid, cv, color=color)
if pdf in self.pdfs_noband:
labels.append(pdf.label)
Expand Down Expand Up @@ -1178,8 +1174,7 @@ def draw(self, pdf, grid, flstate):
labels = flstate.labels
handles = flstate.handles
ax = flstate.ax
next_prop = next(ax._get_lines.prop_cycler)
color = next_prop['color']
color = ax._get_lines.get_next_color()
stats = grid.select_flavour(flstate.flindex).grid_values
gv = stats.data
ax.plot(grid.xgrid, gv.T, alpha=0.2, linewidth=0.5, color=color, zorder=1)
Expand Down
3 changes: 1 addition & 2 deletions validphys2/src/validphys/plotutils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,7 @@ def kde_plot(a, height=0.05, ax=None, label=None, color=None, max_marks=100000):

a = np.asarray(a).ravel()
if color is None:
next_prop = next(ax._get_lines.prop_cycler)
color = next_prop["color"]
color = ax._get_lines.get_next_color()
kde_func = stats.gaussian_kde(a, bw_method="silverman")
kde_x = np.linspace(*expand_margin(np.min(a), np.max(a), 1.3), 100)
ax.plot(kde_x, kde_func(kde_x), label=label, color=color)
Expand Down