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

Multiweight implementation #205

Draft
wants to merge 109 commits into
base: substructure
Choose a base branch
from
Draft

Multiweight implementation #205

wants to merge 109 commits into from

Conversation

jackaraz
Copy link
Member

@jackaraz jackaraz commented Jun 27, 2023

Context:
This PR implements the treatment of multiple weights originating from scale and PDF uncertainties.

Description of the Change:

Readers, cutflow and histogramming objects have been updated to accommodate more than one weight.

Benefits:

This will allow the implementation of theoretical uncertainties into the analysis

Major changes and limitations to backwards compatibility

InitializeForNewEvent function which takes MAfloat64 event weight value, is deprecated. It is an empty function which does not do anything at the moment. This has been replaced by void InitializeForNewEvent(const WeightCollection &EventWeight) function, which takes weight collection as an input. MCEventFormat no longer has a single weight value; everything is controlled by WeightCollection class.

Weights are initialised within main.cpp function. The user does not need to initialise them within the analysis.

@jackaraz jackaraz changed the base branch from main to substructure June 27, 2023 14:20
@github-actions
Copy link

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@jackaraz jackaraz requested a review from BFuks June 28, 2023 11:10
sumweight_positive_ += std::abs(weight);
else
sumweight_negative_ += std::abs(weight);
}

Copy link
Member

Choose a reason for hiding this comment

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

The method addWeightedEvents does not seem to be used anymore. Should we get rid of it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this writes total weights in the main SAF file as a summary, we can extend it with weight collections

Copy link
Member

Choose a reason for hiding this comment

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

I agree with the proposed extension

if not self.has_pdf_unc:
return [self.weights * self.scale] * 2

method = "replicas" # "eigenvector" #
Copy link
Member Author

Choose a reason for hiding this comment

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

@BFuks both implementations are ready to be tested and it can be switched from here (until we hardcode which pdfset uses which method)

* update version
* bugfix probably occured during branch merging
* add control for multiple pdf sets (not available atm)
* minor changes for retreiving items
@jackaraz jackaraz requested a review from BFuks January 11, 2024 15:34
f"{central_pdfs[weight_set[0].pdfset].name} will be used instead"
)

return weight_set[0]
Copy link
Member

Choose a reason for hiding this comment

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

Can't we compute them independently and store all errors? Like for the multiple scale choice possibilities?

Copy link
Member Author

Choose a reason for hiding this comment

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

we can, but lets go step by step please. First we need to figure which method to use for each pdfset then I can look into it.

Copy link
Member

@BFuks BFuks left a comment

Choose a reason for hiding this comment

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

This looks good. Let's now test it on true events :-)

@jackaraz jackaraz added the NormalMode Normal Mode label Jan 12, 2024
@jackaraz jackaraz requested a review from BFuks January 15, 2024 16:08
# wrong so we need to correct them. Some of the files includes
# AUX tag for every weight definition. If the name is AUX_XXX
# where XXX is integer, that is a true aux, if not remove the
# AUX tag.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rewrite as:

        # Check the weight name and make sure its written according
        # to standard conventions, and update it if necessary. This is
        # needed due to terribly written HEPMC files by MadSTR.
        # These files include an `AUX` tag for every weight name. 
        # Therefore, if the weight name is `AUX_XXX`with XXX being
        # an integer, then we have a true `AUX` weight. Otherwise, we
        # remove the `AUX` string from the weight name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. You can use "make a suggestion" option when checking, that automatically creates a comit for the portion of the code that you want to change. Will be more efficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚙️enhancement New feature or request NormalMode Normal Mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants