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

Add truncation in TrackletProcessorDisplaced #293

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

cgsavard
Copy link

@cgsavard cgsavard commented Oct 2, 2024

PR description:

Truncation has never been added to the TrackletProcessorDisplaced module and so these changes implement truncation there using the maxstep_['TP'] value in Settings.h here (as the other displaced modules do). Truncation in extended tracking is turned off by default and so this doesn't change the performance of anything, but allows us to properly study truncation for the full extended chain.

PR validation:

After running a couple of checks, the behavior of these changes is exactly as expected. With truncation off, nothing changes. With truncation on, TrackletProcessorDisplaced is capping its processing of objects to the value set in Settings.h.

@aehart
Copy link

aehart commented Oct 4, 2024

@cgsavard The only thing I would suggest is adding a separate "TPD" key to the maxstep_ map here:

std::unordered_map<std::string, unsigned int> maxstep_{
{"IR", 156}, //IR will run at a higher clock speed to handle
//input links running at 25 Gbits/s
//Set to 108 to match firmware project 240 MHz clock
{"VMR", 107},
{"TE", 107},
{"TC", 108},
{"PR", 108},
{"ME", 108},
//NOTE: The MC is set to 108, but `mergedepth`
//removes 3 iterations to emulate the delay
//due to the HLS priority encoder
{"MC", 108},
{"TB", 108},
{"MP", 108},
{"TP", 108},
{"TRE", 108},
{"DR", 108}}; //Specifies how many tracks allowed per bin in DR

And then use that instead of "TP" in the code you've added. I can see it being useful being able to turn truncation on and off for the prompt and displaced modules separately.

@cgsavard
Copy link
Author

cgsavard commented Oct 5, 2024

The only thing I would suggest is adding a separate "TPD" key to the maxstep_ map

Done!

@aehart
Copy link

aehart commented Oct 7, 2024

The only thing I would suggest is adding a separate "TPD" key to the maxstep_ map

Done!

Thanks, @cgsavard! But you should also change "TP" to "TPD" in TrackletProcessorDisplaced.cc, in order to use the new key in maxstep_, and I think you accidentally changed the default algorithm in L1TrackNtupleMaker_cfg.py to HYBRID_DISPLACED. That's probably why the CI is now failing.

@cgsavard
Copy link
Author

cgsavard commented Oct 7, 2024

@aehart Ah yes, I accidentally added the wrong file to the commit. I just fixed the commit so hopefully everything is good now!

Copy link

@aehart aehart left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll let @tomalin merge this, in case he has any comments.

@tomalin tomalin merged commit aac45c2 into L1TK-dev-14_0_0_pre2 Oct 8, 2024
1 check passed
tomalin pushed a commit that referenced this pull request Oct 11, 2024
* include truncation in TrackletProcessorDisplaced

* code formatting

* include separate truncation for TP and TPD
tomalin pushed a commit that referenced this pull request Oct 29, 2024
* include truncation in TrackletProcessorDisplaced

* code formatting

* include separate truncation for TP and TPD
dally96 pushed a commit that referenced this pull request Dec 3, 2024
* include truncation in TrackletProcessorDisplaced

* code formatting

* include separate truncation for TP and TPD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants