-
Notifications
You must be signed in to change notification settings - Fork 13
Improved softkiller examples #193
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,27 @@ | ||
| # JetReconstruction.jl Examples - SoftKiller | ||
|
|
||
| Here are simple examples of SoftKiller that read from two HepMC3 files and | ||
| then run the SoftKiller algorithm. | ||
|
|
||
| ## `softkiller_plots.jl` | ||
|
|
||
| This example can be run as: | ||
|
|
||
| ```sh | ||
| julia --project softkiller_plots.jl --maxevents=100 --grid-size=0.4 --algorithm=Kt --pileup-file=../../test/data/sk_example_PU.hepmc.zst --hard-file=../../test/data/sk_example_HS.hepmc.zst | ||
| julia --project softkiller_plots.jl --pileup-maxevents=100 --eventno=4 --grid-size=0.4 --algorithm=Kt ../../test/data/sk_example_HS.hepmc.zst ../../test/data/sk_example_PU.hepmc.zst | ||
| ``` | ||
|
|
||
| This is a simple example of SoftKiller that reads from two HepMC3 files | ||
| and displays plots of clustering without SoftKiller and with SoftKiller. | ||
| and displays plots of clustering with SoftKiller and without SoftKiller. | ||
|
|
||
| ## `softkiller_runtime.jl` | ||
|
|
||
| This example can be run as: | ||
|
|
||
| ```sh | ||
| julia --project softkiller_runtime.jl --maxevents=100 --grid-size=0.4 --algorithm=Kt --pileup-file=../../test/data/sk_example_PU.hepmc.zst --hard-file=../../test/data/sk_example_HS.hepmc.zst | ||
| julia --project softkiller_runtime.jl --pileup-maxevents=100 --eventno=4 --grid-size=0.4 --algorithm=Kt ../../test/data/sk_example_HS.hepmc.zst ../../test/data/sk_example_PU.hepmc.zst | ||
| ``` | ||
|
|
||
| This is a simple example of SoftKiller that reads from two HepMC3 files | ||
| and displays the runtime of the SoftKiller algorithm. | ||
| For both scripts the number of pileup events to add to the hard scatter event | ||
| can be controlled with `--pileup-maxevents` and `--pileup-skip`; and the hard | ||
| scatter event to work with can be changed with `--eventno`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #! /bin/sh | ||
|
|
||
| # Plain softkiller example | ||
| julia --project softkiller_runtime.jl --pileup-maxevents=100 --eventno=4 --grid-size=0.4 --algorithm=Kt ../../test/data/sk_example_HS.hepmc.zst ../../test/data/sk_example_PU.hepmc.zst | ||
|
|
||
| # Softkiller with graphics | ||
| julia --project softkiller_plots.jl --pileup-maxevents=100 --eventno=4 --grid-size=0.4 --algorithm=Kt ../../test/data/sk_example_HS.hepmc.zst ../../test/data/sk_example_PU.hepmc.zst |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is for a future PR, but I wanted to say that I find this procedure to be very cumbersome, in-part because it has to happen between applying filters to the input constituents and any jet finding that is done later.
SK isn't the only filter people will want to study: CS and CHS are also obvious things to look at, and grooming jets will have a similar problem.
Is there some smarter way to handle the cluster_hist_index assignment when filtering particles? In FastJet I guess that this is part of the
ClusterSequenceand so it's handled there instead of by the user ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both in Fastjet and JetReconstruction the cluster history indices are part of the jet classes. The problem here is that our
PseudoJets are Julia's struct and are immutable so their fields (index) can't be updated later so we have to create a new jet object with different index. Prior to #146, we hadmutable structwhich can be modified but had a number of other annoyances (reasoning about code with constants , less performant, more awkward C-bindings)I guess as a minimum we could add a function taking a vector of jets after filter and returning vector of jets with normalized indices or maybe doing this inplace
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @mattleblanc
Your point is very well taken. We started to put effort into ensuring that this would be correct, but there are many cases to cover and lots of different paths to failure!
I propose that the reconstruction algorithms themselves take responsibility for checking and fixing things if a collection is passed that has mis-set the
cluster_hist_index. This should not cost much at runtime and it makes things much safer and prevents the code from bombing if there's a filtering/selection path that didn't fix-up thecluster_hist_indexitself.See #200.
AFAICR Fastjet is using mutable structures, which is inherently more dangerous for the code, but allows these kinds of fix-ups on the fly. But we have ways to make sure that this is correct in Julia as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that having the clustering algorithms themselves handle this makes the most sense, even if it costs a little bit. In this case, at least we can ensure it's done as optimally as possible, instead of leaving the job up to the user.
Mutable structs seem undesirable for the reasons you both mention...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also makes things easier in the case of SoftKiller as it can just pass back the current list of jets and not worry about having to apply these fixes - this would make #191 moot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could consider writing custom https://juliaobjects.github.io/Accessors.jl/dev/examples/custom_optics/ if it makes everyone's life easier while keeping strict immutable