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

Draft: Picker24/feature/generic binning and return kinematic parameter tidy #169

Conversation

luketpickering
Copy link
Contributor

Pull request description

changes ahead of new MaCh3 core merge.

Changes or fixes

  • adds new 'generic' ND binning option.
    • Currently only N independent contiguous (uniform or variable width) axes, the ND binning is the simple product of these N axes.
  • simplifies ReturnKinematicParameter[ByReference] interface
  virtual const double &
  ReturnKinematicParameterByReference(int KinematicParameter, int iSample,
                                      int iEvent) = 0;
  // LP - use this to implement kinematic parameters that are not doubles (so we
  // cannot return the address of one), but can be cast to doubles, note that this
  // will likely cause extreme confusion and the fact we might need this
  // indicates an issue in our abstraction. Parameters defined in
  // implementations of this method can not be used as projection variables,
  // which are kept as pointers to doubles.
  virtual double ReturnKinematicParameter(int KinematicParameter, int iSample,
                                          int iEvent) = 0;

Examples

New ND binning option

Binning:
  Axes:
    - VarStr: "RecoNeutrinoEnergy" 
      VarBins: [0.,  0.5,  1.,  1.25, 1.5, 1.75, 2., 2.25, 2.5, 2.75, 3., 3.25, 3.5, 3.75, 4., 5., 6., 10.]
    - VarStr: "ERecQE" 
      Uniform: [10, 0, 10]
    - VarStr: "EHadRec" 
      Uniform: [10, 0, 10]

Luke Pickering added 3 commits October 16, 2024 11:10
- also removes useless inlines
Slim down to two virtual functions. Now the job of the caller to convert
string parameter names to subclass-specific integer identifiers.

ReturnKinematicParameterByReference now actually returns by reference.
Adds a new format for binning definitions:

```yaml
Binning:
  Axes:
    - VarStr: "RecoNeutrinoEnergy"
      VarBins: [0.,  0.5,  1.,  1.25, 1.5, 1.75, 2., 2.25, 2.5, 2.75,
                3., 3.25, 3.5, 3.75, 4., 5., 6., 10.]
    - VarStr: "ERecQE"
      Uniform: [10, 0, 10]
    - VarStr: "EHadRec"
      Uniform: [10, 0, 10]
```

To the rest of MaCh3 this will appear like a 1D analysis, where the
analysis variable is the global bin number. When using this option,
the XVarStr is set to "global_bin_number", which must be implemented
by all SamplePDF sub classes that want to use this feature. We can
integrate this into Core more directly in the future, but for now,
I think this works.

'flow bins in the ND binning are ignored, the 1D binning runs from
[-0.5,nbins-0.5]. The 'default' ROOT 'flow bins exist on the 1D binning
and so the global bin finder method returns a value of -1 for a over/under
flow on any individual axis.
Copy link

Hi, @luketpickering,
Thanks for opening this PR 💙 .
Contributors 🧑‍🤝‍🧑 like you make the open source community 🌍 such an amazing place to learn 📖 , inspire 👼, and create 🎨 .
We will review it 👀 and get back to you as soon as possible 👍 . Just make sure you have followed the contribution guidelines.

By that time enjoy this meme 👇 , hope you like it 😄

meme

Use this action on your projects. Use jokes on issues instead.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is your first PR, thank you for contributing to MaCh3!

Luke Pickering added 4 commits October 16, 2024 13:39
@github-actions github-actions bot added Nu Osc/Xsec Related with neutrino interactions or oscialtions Cmake labels Oct 16, 2024
- slice function implementations on the way!
- probably this is now enough 'binning' code that it shouldn't just live
in SamplePDF
…face

- to view/interrogate histograms, 1D global bin number histograms are useless
see samplePDF/GenericBinningTools.h for interface
@luketpickering luketpickering changed the title Picker24/feature/generic binning and return kinematic parameter tidy Draft: Picker24/feature/generic binning and return kinematic parameter tidy Oct 17, 2024
Base automatically changed from feature_OscClassMagic to develop October 17, 2024 15:50
Luke Pickering added 3 commits October 18, 2024 13:53
@KSkwarczynski
Copy link
Member

CI which has ROOT compiled with c++14 is failing:
https://github.com/mach3-software/MaCh3/actions/runs/11407213472/job/31742713856?pr=169

Which is related to bumping c++ standard.

This isn't no stopper but probably should discuss on gneral meeting if everyone is fine or if there are poeple who will be affected by this change etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmake Nu Osc/Xsec Related with neutrino interactions or oscialtions Samples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants