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

Clean up TrackletConfigBuilder #90

Open
tomalin opened this issue Sep 16, 2021 · 0 comments
Open

Clean up TrackletConfigBuilder #90

tomalin opened this issue Sep 16, 2021 · 0 comments
Assignees

Comments

@tomalin
Copy link
Collaborator

tomalin commented Sep 16, 2021

=== MAIN COMMENTS ===

  1. Physics question: The hourglass phi width varies vs radius, whereas the code in initGeom() ignores this. Is it wasting FPGA resources by creating more VM & coarse regions than needed?

Ryd: It is correct that this will create VM regions that are not used, and in general the phi region have an uneven load. In principle one could adjust these such that the 'hourglass' shape is respected. However, it will lead to a number of complication where more significant code changes will be needed in essentially all modules (VMR, TP, MP for the combined modules) to handle this. Though I have not studied this carefully, I think that the gain from optimizing the phi ranges would be lost by the extra complications in the code.

  1. Physics question: What is motivation in "disks of r1 = rmean[0] + 40" in seedRadii() ? Please add comment explaining it and eliminate magic number "40".

Ryd: This is an approximation of the larger radius to consider when forming stub pairs - I will look into replacing this with a calculated number based on other constants.

  1. buildTC() needs clearer comments explaining physics of what it's doing. I don't understand TEs (TrackletEngines?) being grouped into TCs (TrackletCalculators).

Ryd: Each TE (TrackletEngine) that forms candidate stub pairs are the input to one and only one TC (or TP). This groups the TEs that are processed by one TC.

  1. buildProjections() contains a hard-wired list of empty projections. A comment should be added explaining how these lists are obtained.

Ryd: Agree - I will explain how this is done.

=== STYLE ===

  1. To me "Config" suggests CMSSW configuration parameters. I therefore suggest renaming the class to "WiringBuilder".

Ryd: I don't think this is something we should spend time on, and I don't think CMSSW should monopolize words like Config...

=== EXTRA FEATURE ===

  1. It should be able to produce a reduced configuration, e.g. for the skinny chain. This would mean that when generating txt files for the HLS for skinny chain, it would not need to read the wiring map from an external wires.dat file, where the latter is produced by running https://github.com/cms-L1TK/project_generation_scripts/blob/master/makeReducedConfig.py on the full wires.dat file.
    This would require convering makeReducedConfig.py to C++, so it could be run in CMSSW after TrackletConfigBuilder has produced the full wiring map.

Ryd: Not sure that this is the right way to go. The skinny chains are useful now, but I think that in the long term we should have the main code not bugged down by these 'debugging and development' tools.

=== MERGE ===

  1. A variant of TrackletConfigBuilder exists in the Future L1 track Emulation code. They should one day be merged.
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

No branches or pull requests

2 participants