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

Iss864 #865

Merged
merged 6 commits into from
May 26, 2021
Merged

Iss864 #865

merged 6 commits into from
May 26, 2021

Conversation

normangraf
Copy link
Contributor

I removed superfluous code in ReconParticleDriver that accessed the rotated helical track hits for no reason.
One can now run the Kalman Filter track finding and fitting without having to create HelicalTrackHits.
The KF only steering file has been modified to remove that Driver.
It has also been updated to use the distance-based track cluster matcher.
It also now keeps monster events instead of zeroing out the 1D strip hit collection.

Graf added 6 commits May 20, 2021 21:20
Remove HelicalTrackHitDriver as it should not be needed for the Kalman Filter.
Remove unused calls to  hitToStrips and hitToRotated
Switch to MinDist track cluster matcher
Don't skip monster events.
@cbravo135
Copy link
Collaborator

Can you show some checks that this did not change the behavior of PhysicsRun2019_pass0_recon_evio.lcsim?

@normangraf
Copy link
Contributor Author

The two collections that were being accessed were never being used. Commenting out those calls should not affect the results of any other Drivers.

@@ -87,7 +89,7 @@

<driver name="TrackerHitDriver" type="org.hps.recon.tracking.DataTrackerHitDriver">
<neighborDeltaT>8.0</neighborDeltaT>
<saveMonsterEvents>false</saveMonsterEvents>
<saveMonsterEvents>true</saveMonsterEvents>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this changed from false to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe the current way of handling these events is correct, and simply skipping these events may be giving us a false impression of how well we're doing. I'd like to see what effect this has in combination with enabling the event flag filter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The event flag filter is not currently activated in this steering file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is your intention, please add it to this steering file and I will approve this PR.

@cbravo135
Copy link
Collaborator

I think at a minimum you should run on one file and show that these collections are indeed removed as expected.

@cbravo135
Copy link
Collaborator

Also, if we plan on moving forward with this steering file now, you should add the EventFlagFilter driver and turn on the "svt_readout_overlap_good" flag.

@normangraf
Copy link
Contributor Author

normangraf commented May 25, 2021

Seems to be a "Fitting with: Simplex" debug printout left in the code.

Ooops! Added this comment to the wrong PR.

@normangraf
Copy link
Contributor Author

The collections aren't (yet) being removed. I was simply removing an extraneous call to access those collections, which requires them to be in the event. Once we drop the SeedTracker and GBL track finding and fitting these collections will no longer be in the event. No behavior has changed.

@pbutti pbutti merged commit 263d9ca into master May 26, 2021
@pbutti pbutti deleted the iss864 branch May 26, 2021 18:36
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants