search

cms-L1TK / project_generation_scripts

Python scripts to generate the wiring map of the tracklet pattern recognition & the top-level HDL that calls the HLS modules in the Hybrid Chain.
1 stars 2 forks source link

Updated phibinword for IR #37

Closed meisonlikesicecream closed 2 years ago

meisonlikesicecream commented 2 years ago

According to: https://github.com/cms-L1TK/firmware-hls/blob/reduced_config_IR_checked/TestBenches/InputRouter_test.cpp

sseifeln commented 2 years ago

it looks correct to me : 32 bit word for each IR; 8 bits for each layer ; 1 bit per phi bin 👍

meisonlikesicecream commented 2 years ago

@sseifeln We merge it once your changes to the firmware-hls repo has been merged as well?

sseifeln commented 2 years ago

@meisonlikesicecream - makes sense to me - I am creating the PRs to the two branches now

tomalin commented 2 years ago

According to: https://github.com/cms-L1TK/firmware-hls/blob/reduced_config_IR_checked/TestBenches/InputRouter_test.cpp

Please explain in more detail what this PR is for. Also do I understand that this PR must not be merged until https://github.com/cms-L1TK/firmware-hls/pull/194 is merged?

meisonlikesicecream commented 2 years ago

According to: https://github.com/cms-L1TK/firmware-hls/blob/reduced_config_IR_checked/TestBenches/InputRouter_test.cpp

Please explain in more detail what this PR is for. Also do I understand that this PR must not be merged until cms-L1TK/firmware-hls#194 is merged?

In that PR Sarah has changed how the phibinword is structured, and this PR is just to make the same change but in the python scripts. Instead of counting the number of bins per layer and saving that number using three bits (i.e. max 8 memories), there is 8 bits per layer and each bit represent one of the 8 phi regions A-H; a 0 means the memory is not used, a 1 means it is.

tomalin commented 2 years ago

@meisonlikesicecream today's HLS decided that https://github.com/cms-L1TK/firmware-hls/pull/194 will not be merged into "master". Instead it is merged into the "reduced" branch, which will in a few weeks be merged into "master". Therefore can we merge this PR into "master" now, or must we wait until "reduced" is merged into master?

meisonlikesicecream commented 2 years ago

@meisonlikesicecream today's HLS decided that cms-L1TK/firmware-hls#194 will not be merged into "master". Instead it is merged into the "reduced" branch, which will in a few weeks be merged into "master". Therefore can we merge this PR into "master" now, or must we wait until "reduced" is merged into master?

It depends on if you want the project generation scripts to be incompatible with the firmware-hls master branch or not. Might be confusing if someone would want to create a VHDL top-level using the current IR in master, but I don't know if anyone would actually do that since most people are working on the "reduced" branch.

tomalin commented 2 years ago

Understood. I recommend that we delay merging this branch into "master" until the HLS branch "reduced_config_pr" has been merged into HLS "master" (a few weeks from now). To check if this merge has happened, at least for IR parts of code, is to check if TestBenches/InputRouter_test.cpp in "master" contains function "getMap()". Not yet the case.

tomalin commented 2 years ago

I have just rebased this (I hope) to the latest master, into which was just merged https://github.com/cms-L1TK/project_generation_scripts/pull/38 . The recipe I used for this was:

  1. git clone -b ir_phibinword_update git@github.com:cms-L1TK/project_generation_scripts.git
  2. git checkout master
  3. git checkout ir_phibinword_update
  4. git rebase master
  5. git push -f origin ir_phibinword_update