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

Photon Gradient Boosted Tree Filter #216

Merged
merged 48 commits into from
May 28, 2024
Merged

Conversation

Gregtom3
Copy link
Contributor

@Gregtom3 Gregtom3 commented May 17, 2024

Photon GBT Filter

Description

At CLAS12, neutral particles such as photons are often falsely reconstructed. For instance, events without a true SIDIS photon, say, from a neutral pi0 decay, will still contain reconstructed photons in the REC::Particle bank. These photons are false backgrounds for many physics channels. A new algorithm titled PhotonGBTFilter was added to the repository to filter out this false photon background.

Models trained on RGA inbending, RGA outbending, and RGC Monte Carlo were produced. Future plans are to include models for RGK, as well as distinct models for pass1 vs. pass2 cooking.

A validator algorithm was also provided in the PR. This algorithm can be run by...
meson test validator-clas12-PhotonGBTFilter --verbose --test-args '\-f /cache/clas12/rg-a/production/recon/fall2018/torus-1/pass1/v1/dst/train/nSidis/nSidis_005032.hipo \-n 0 \-o ../validator_output' ...below is the output
download

@Gregtom3 Gregtom3 requested a review from c-dilks as a code owner May 17, 2024 17:48
@c-dilks
Copy link
Member

c-dilks commented May 17, 2024

Please add yourself and your code to the CODEOWNERS file (it's how we keep track of who maintains the algorithms)

Gregtom3 and others added 8 commits May 20, 2024 11:32
- renamed: src/iguana/algorithms/clas12/PhotonGBTFilter.cc                      -> src/iguana/algorithms/clas12/PhotonGBTFilter/Algorithm.cc
- renamed: src/iguana/algorithms/clas12/PhotonGBTFilter.h                       -> src/iguana/algorithms/clas12/PhotonGBTFilter/Algorithm.h
- renamed: src/iguana/algorithms/clas12/PhotonGBTFilter.yaml                    -> src/iguana/algorithms/clas12/PhotonGBTFilter/Config.yaml
- renamed: src/iguana/algorithms/clas12/PhotonGBTFilterValidator.cc             -> src/iguana/algorithms/clas12/PhotonGBTFilter/Validator.cc
- renamed: src/iguana/algorithms/clas12/PhotonGBTFilterValidator.h              -> src/iguana/algorithms/clas12/PhotonGBTFilter/Validator.h
- renamed: src/iguana/algorithms/machine_learning/photon_gbt/RGA_inbending.cpp  -> src/iguana/algorithms/clas12/PhotonGBTFilter/models/RGA_inbending.cpp
- renamed: src/iguana/algorithms/machine_learning/photon_gbt/RGA_outbending.cpp -> src/iguana/algorithms/clas12/PhotonGBTFilter/models/RGA_outbending.cpp
- renamed: src/iguana/algorithms/machine_learning/photon_gbt/RGC_Summer2022.cpp -> src/iguana/algorithms/clas12/PhotonGBTFilter/models/RGC_Summer2022.cpp
@dglazier
Copy link
Contributor

A couple of comments relating to integration with iguana. First the passing of the run number into the action would probably be best handled elsewhere in a manner used by all algorithms, it is a general issue. For example in a proposed ChangeRun function. In general this gives the action function less to do. Related, it looks like the models would benefit from class inheritance. You could then just construct the valid model for the run number in the ChangeRun function and avoid needing to reproduce code in the models. This would make is simpler to expand to further beam periods in the future. Alternatively it may be possible to move all the parameters into configuration files and just load them via the yaml readers and just require 1 model class at ChangeRun time to avoid any hard-coding. Or is there a reason why this would not work ?
Cheers
Derek

Copy link
Member

@c-dilks c-dilks left a comment

Choose a reason for hiding this comment

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

Some general comments, for future consideration:

  • std::unordered_map lookups are rather slow, and should be avoided in the Run method or action functions where possible
  • for compatibility with the diverse set of user analysis tools, action functions are only allowed to use basic parameter types, such as int and float; these are scalar action functions, and in the future we will also have vector action functions, which accept std::vector<int> etc. It looks like this algorithm should have a vector action function

CODEOWNERS Outdated Show resolved Hide resolved
CODEOWNERS Outdated Show resolved Hide resolved
examples/iguana-example-photon-ML.cc Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
src/iguana/algorithms/Algorithm.h Outdated Show resolved Hide resolved
src/iguana/algorithms/clas12/PhotonGBTFilter/Validator.h Outdated Show resolved Hide resolved
src/iguana/algorithms/clas12/PhotonGBTFilter/Validator.cc Outdated Show resolved Hide resolved
src/iguana/algorithms/clas12/PhotonGBTFilter/Validator.cc Outdated Show resolved Hide resolved
src/iguana/algorithms/clas12/PhotonGBTFilter/Validator.cc Outdated Show resolved Hide resolved
examples/meson.build Outdated Show resolved Hide resolved
@c-dilks
Copy link
Member

c-dilks commented May 21, 2024

First the passing of the run number into the action would probably be best handled elsewhere in a manner used by all algorithms, it is a general issue. For example in a proposed ChangeRun function. In general this gives the action function less to do.

@dglazier let's leave Gregory's run-dependent implementation as is here, and followup in #95.

Gregtom3 and others added 21 commits May 22, 2024 11:45
Co-authored-by: Christopher Dilks <[email protected]>
Co-authored-by: Christopher Dilks <[email protected]>
Co-authored-by: Christopher Dilks <[email protected]>
@c-dilks c-dilks mentioned this pull request May 28, 2024
@c-dilks
Copy link
Member

c-dilks commented May 28, 2024

CI results:
photon_gbt_plot

@c-dilks c-dilks merged commit bb81fe9 into JeffersonLab:main May 28, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants