Adding action to plot chi2 dist for aggregate of datasets

[siranipour]

Closes #687

Yet to work on the KDE business. Example runcard, mainly for myself, because it's currently in /tmp

pdf: NNPDF31_nnlo_as_0118_DISonly
fit: NNPDF31_nnlo_as_0118_DISonly

experiments:
  from_: fit

theoryid: 53

use_cuts: fromfit

template_text: |
  {@plot_chi2dist_experiments@}

actions_:
  - report(main=True)

[siranipour]

This will deprecate

nnpdf/validphys2/src/validphys/dataplots.py

Line 32 in 7eb1089

def plot_chi2dist(results, dataset, abs_chi2_data, chi2_stats, pdf):

right?

[siranipour]

Note to self: for some reason the std_error is a vector of 0s, will work on this later. Problem is probably the same problem as why we need squeeze()

[Zaharid]

This will deprecate

nnpdf/validphys2/src/validphys/dataplots.py

Line 32 in 7eb1089

def plot_chi2dist(results, dataset, abs_chi2_data, chi2_stats, pdf):

right?

I think it it is good to have an action that works on a single dataset, for convenience.

[Zaharid]

@siranipour is this working now?

[siranipour]

Well kinda, I just don't why the std_error was a vector of 0s. Haven't looked at this since the previous commit

[siranipour]

This will deprecate

nnpdf/validphys2/src/validphys/dataplots.py

Line 32 in 7eb1089

def plot_chi2dist(results, dataset, abs_chi2_data, chi2_stats, pdf):

right?

I think it it is good to have an action that works on a single dataset, for convenience.

Btw why is results a provider of that action? It's unused.

[Zaharid]

Looks like a bug.

[siranipour]

Looks like a bug.

Will remove in this PR

[siranipour]

This now works, there was a bug in total_experiments_chi2data which I've now fixed thanks to a comment by Nathan back in 2018 at 18.30 (what a hard worker).

[siranipour]

I'll see what I can do re the KDE plots and hopefully have this merged soon

[siranipour]

Ok KDE plots added, let me know if it's what you had in mind. This should be ready to merge.

You can get the KDE plot using

pdf: NNPDF31_nnlo_as_0118_DISonly
fit: NNPDF31_nnlo_as_0118_DISonly

experiments:
  from_: fit

theoryid: 53

use_cuts: fromfit

template_text: |
  {@kde_chi2dist_experiments@}

actions_:
  - report(main=True)

[Zaharid]

The KDE plot could use some decoration such as titles, axis labels, and maybe the same sort of legend that the histogram has.

[siranipour]

Done

[wilsonmr]

looks nice - another thing that will need to be changed with #651 we should really get that finished off so that people can start using the new interface.

[wilsonmr]

Given that the code for plot_chi2dist and plot_chi2dist_experiments is almost identical - I wonder if it'd be neater to have the plotting code in a function which is then called by plot_chi2dist and plot_chi2dist_experiments because as far as I can tell the only difference is the title?

The KDE code also looks pretty similar - I wonder if we could have the same base code accept a keyword whether to use hist or KDE? I guess there seems to be a lot of unneccesary boilerplate here which could be put in some base function

EDIT: sorry I mixed up the funcitons I was referring to in original comment - should make sense now

[siranipour]

But plot_chi2dist and plot_experiments_chi2dist have different providers

[wilsonmr]

but look:

def base_function(experiment_or_dataset_chi2, stats, pdf):
    label = pdf.label # should we use label or name here?
    fig, ax = plt.subplots()
    alldata, central, _ = experiment_or_dataset_chi2
    if not isinstance(alldata, MCStats):
        ax.set_facecolor("#ffcccc")
        log.warning("Chi² distribution plots have a "
                "different meaning for non MC sets.")
        label += " (%s!)" % pdf.ErrorType

    label += '\n'+ '\n'.join(str(chi2_stat_labels[k])+(' %.2f' % v) for (k,v) in stats.items())
    ax.set_xlabel(r"Replica $\chi^2$")
    ax.hist(alldata.data, label=label, zorder=100)
    l = ax.legend()
    l.set_zorder(1000)
    return fig, ax

@figure
def plot_chi2dist_experiments(total_experiments_chi2data, experiments_chi2_stats, pdf):
    fig, ax = base_function(total_experiments_chi2data, experiments_chi2_stats)
    ax.set_title(r"Experiments $\chi^2$ distribution")
    return fig

trivial to add another action for dataset one and not that difficult to make the base function do either KDE or histogram

[wilsonmr]

as per my old code - should we use pdf.label or pdf.name here?

[siranipour]

Doesn't look like there is a difference on the examples I tried.

[wilsonmr]

I think it would make a difference if you declared PDF like in vp-comparefits however

pdf: {id: id_of_the_base_fit, label: "whatever you like"}

[siranipour]

but look:

def base_function(experiment_or_dataset_chi2, stats, pdf):
    label = pdf.label # should we use label or name here?
    fig, ax = plt.subplots()
    alldata, central, _ = experiment_or_dataset_chi2
    if not isinstance(alldata, MCStats):
        ax.set_facecolor("#ffcccc")
        log.warning("Chi² distribution plots have a "
                "different meaning for non MC sets.")
        label += " (%s!)" % pdf.ErrorType

    label += '\n'+ '\n'.join(str(chi2_stat_labels[k])+(' %.2f' % v) for (k,v) in stats.items())
    ax.set_xlabel(r"Replica $\chi^2$")
    ax.hist(alldata.data, label=label, zorder=100)
    l = ax.legend()
    l.set_zorder(1000)
    return fig, ax

@figure
def plot_chi2dist_experiments(total_experiments_chi2data, experiments_chi2_stats, pdf):
    fig, ax = base_function(total_experiments_chi2data, experiments_chi2_stats)
    ax.set_title(r"Experiments $\chi^2$ distribution")
    return fig

trivial to add another action for dataset one and not that difficult to make the base function do either KDE or histogram

Ahhhh I see, two mins will do this now

[siranipour]

Thanks for the suggestion @wilsonmr very good point. I have now refactored accordingly. Let me know what you think.