Skip to content
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

Handling of all PDF uncertainty sets #494

Open
konda111 opened this issue Oct 26, 2021 · 6 comments
Open

Handling of all PDF uncertainty sets #494

konda111 opened this issue Oct 26, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@konda111
Copy link

Hi,

We tried using adding the pdf uncertainty as a systematic in our Madminer setup. We used the MSTW pdf set and the systematic was not recognized after the MG run.

We think that the block

if (
      "mg_reweighting" in wg_name.lower()
      or "pdf" not in wg_name.lower()
      or "ct" not in wg_name.lower()
      or systematic.value not in wg_name.lower()
  ):

in madminer/utils/interfaces/lhe.py should be extended to allow also for PDF sets not containing the strings pdf or ct.

Best,
Henning

@irinaespejo
Copy link
Member

Hi Henning,

Could you post a minimal example code that throws the error and whatever logs you have? Alternatively, pointing us to your working notebook could also be helpful.

Thank you,

Irina

@irinaespejo irinaespejo added the enhancement New feature or request label Oct 26, 2021
@konda111
Copy link
Author

konda111 commented Oct 26, 2021

Hi Irina,

the issue does not produce any error. To encounter it, one has to load a h5 file, which has been generated based on a .lhe file with MG PDF reweighting. Loading it e.g. with

    sampler = SampleAugmenter(
        f"{paths['h5_sample_file']}",
        include_nuisance_parameters=True,
    )

one then encounters something like

13:44 madminer.analysis.da INFO    Found 1 nuisance parameters
13:44 madminer.analysis.da INFO      0: scale (scale / mu / 0.5,1.0,2.0)

in the log, whereas the output should be something like

13:44 madminer.analysis.da INFO    Found 41 nuisance parameters
13:44 madminer.analysis.da INFO      0: scale (scale / mu / 0.5,1.0,2.0)
13:44 madminer.analysis.da INFO      1: pdf (pdf / errorset)

which we got after fixing this issue in a private fork of MadMiner. The issue lies in the function extract_nuisance_parameters_from_lhe_file which is called by the DelphesReader and/or the LHEReader.

Best,
Henning

P.S.: I would not regard this as an enhancement. For me, this is clearly a bug, since it is nowhere documented that systematic PDF uncertainties are only supported for PDFs with a name incorporating pdf or ct.

@Sinclert
Copy link
Member

Hi @konda111

Thanks for opening this issue 😄

I understand this topic lays in a some-how grey area between bug and incomplete documentation. The if block you mentioned in your issue description was introduced by Johann in this 2019 commit, only to be refactored by me very recently.

I would like to understand what other PDF weight names could we expect coming from a .lhe file, in order to include them in the list of found nuisance parameters.

Could you point us to the fix in your private fork?

@konda111
Copy link
Author

Hi,

in our private fork, we just added mwst as a possible identifier, since we used the MWST PDF set. This is of course only a partial solution, since there are many more possible names. Since MG normally uses LHAPDF for accessing PDF sets, working with the list at https://lhapdf.hepforge.org/pdfsets.html could be an option for you.

Btw, the identifier ct was probably include, since there are CT PDF sets; the identifier pdf for the PDF4LHC PDF sets.

Best,
Henning

@Sinclert
Copy link
Member

I see.

Our desirable fix would be to include an standardized list of prefixes / list of names, so that we should not need to open a similar issue in the future about why PDF set <introduce-name-here> was not supported in the first place.

Wondering what is the best approach to tackle this... Any ideas? @johannbrehmer @irinaespejo @matthewfeickert

@Sinclert
Copy link
Member

Sinclert commented Nov 3, 2021

In order to unblock this issue and to encourage the usage of the main package, instead of a custom fork, I am going to add mwst as one of the possible prefixes in this if clock logic. You can expect the change to be available in the upcoming 0.9.2 version.

The issue will remain open for now, as we lack an standardized solution to cover all PDF datasets...

@Sinclert Sinclert changed the title Handling of pdf uncertainty fails if pdf name does not include 'pdf' or 'ct' Handling of all PDF uncertainty sets Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants