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

Files that describe a reduced config for testing a chain of the track finder. #1

Closed
wants to merge 1 commit into from

Conversation

trholmes
Copy link

No description provided.

@tomalin
Copy link

tomalin commented Sep 30, 2021

If I search for "VMSTE_L1PHID16n3" in wires.dat and reduced_wires.dat, then the input seems to have inconsistent format in the two cases: VMR_L1PHID.vmstuboutTEIPHID16 vs VMR_L1PHID.vmstuboutPHID16n3 . What is the motivation for this? It would require different code to process the wiring map depending on whether reduced format is used or not.

@tomalin
Copy link

tomalin commented Sep 30, 2021

I assume that as the summer chain is temporary, there is no intention to push this reduced wiring map to central CMSSW? So if we put it here, we'd have to be careful not to use this repo for updates to the full wiring map, until the summer chain is finished.
For this reason, I think it may be better to push your changes to a branch of this repo. e.g. "reduced_chain"

@trholmes
Copy link
Author

RE the differences in VMSTE_L1PHID16n3 -- In my version of wires.dat, it's the same:

In my version:
lxplus7102:$ grep VMSTE_L1PHID16n3 *wires.dat
reduced_wires.dat:VMSTE_L1PHID16n3 input=> VMR_L1PHID.vmstuboutPHID16 output=> TE_L1PHID16_L2PHIB16.innervmstubin
wires.dat:VMSTE_L1PHID16n3 input=> VMR_L1PHID.vmstuboutPHID16 output=> TE_L1PHID16_L2PHIB16.innervmstubin

The machinery I wrote for doing this is really just copying lines once it figures out the right modules to include. Maybe this changed at some point? Can you tell me which wires.dat you're comparing to?

@trholmes
Copy link
Author

Regarding the temporary nature of the files -- do you think this would be an issue, since they're called reduced_*.dat? We should be able to have both the full and reduced config here simultaneously, and update the reduced_ if we're dealing with a new chain as we scale up.

@tomalin
Copy link

tomalin commented Oct 15, 2021

The summer chain work is (hopefully!) going to be finished within ~2 months. And the earliest we could hope to get your reduced C++ into official CMSSW is ~2 months. So if we merged this trholmes:reduced-configuration data files into the official cms-data repo, it's unlikely that it would ever be used there.

@tomalin
Copy link

tomalin commented Oct 15, 2021

I assume these data files were produced by usig the TrackletConfigBuilder inside the emulation to write out the full wires.dat file (i.e. with Settings::writeConfig_ = true), and then running the HLS script makeReducedConfig.py on this?

In the longer term, could we delete makeReducedConfig.py and add the functionality to generate reduced wires.dat to TrackletConfigBuilder? This would avoid the need to read any reduced wires.dat into CMSSW. If this is possible, then it's another argument for not putting these reduced .dat files into the official CMSSW release. I've added this proposal to cms-L1TK/cmssw#90 .

@tomalin
Copy link

tomalin commented Oct 29, 2021

Since we don't want to merge this into cms-L1TK:master (since the files will not be needed once the summer chain is complete), please move it to a branch such as "summer-chain" in https://github.com/cms-L1TK/L1Trigger-TrackFindingTracklet . And ensure that it is documented somewhere that people generating test-data for the summer chain should use this. For example, in the HLS README.md file after the next fw_sync (an updated version of cms-L1TK/firmware-hls#208 )

@tomalin
Copy link

tomalin commented Oct 29, 2021

@trholmes Can you please take a look at the comments on this PR, so we can decide whether to close it?

@trholmes
Copy link
Author

trholmes commented Nov 1, 2021

Yes, that makes sense -- I'll put together a new PR and close this one.

@trholmes trholmes closed this Nov 1, 2021
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.

2 participants