Test vectors for reduced config #205
Conversation
|
There are data format inconsistencies for the InputRouter test data. e.g. Comparing the first column of MemPrints/InputStubs/InputStubs_IL_L1PHID_PS10G_1_A_04.dat with that in either "reduced_config_pr" or "master", this new PR has values such as "0x02", whereas the older files have instead "02". |
|
I had originally put these TVs directly into a directory here: /eos/project/c/cmsweb/www/upg_trk_l1trg, along with text files that had the git hash corresponding to their production, but I don't see them in the tar'd files, and the structure has been changed in this directory. @aehart, do you know if those text files still exist somewhere? |
|
Ian,
I finally got back to looking at this. It seems that the use of a leading ‘0x’ is not used consistently at all. Seems almost 50/50 (but I have not check all memories). My proposal would be to consistently add the ‘0x’ in all file we write.
Thoughts/suggestions?
-Anders
Anders Ryd
***@***.******@***.***>
On Oct 4, 2021, at 1:09 PM, Ian Tomalin ***@***.******@***.***>> wrote:
There are data format inconsistencies for the InputRouter test data. e.g. Comparing the first column of MemPrints/InputStubs/InputStubs_IL_L1PHID_PS10G_1_A_04.dat with that in either "reduced_config_pr" or "master", this new PR has values such as "0x02", whereas the older files have instead "02".
It was requested in CMSSW issue cms-L1TK/cmssw#85<cms-L1TK/cmssw#85> to add "0x" to this column, so I assume the new test data was added after doing this. (Which branch of our L1Trk CMSSW code was used to do this?). However, FileReaderFIFO.vhd will need a small change to accommodate the extra "0x", so we should make this change for both reduced & non-reduced test data at the same time.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#205 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTIE7JIMN26L2ZECROLUFHNVFANCNFSM5E3WV7CQ>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
Hi Anders,
I agree with your proposal. Though I believe the “0x” is only missing in the input test data to the InputRouter, (i.e. The InputStubs*.dat files). I believe all other test data already includes “0x?
Adding the missing “0x” only requires a 1 line change to our C++. This change would also need making to the fw_sync tag (or a new tag making from the old tag with this change), so that Andrew could ensure that all our HLS test data (both reduced chain and non-reduced chain) include the “0x”. It would also require a 1 line change to FileReaderFIFO.vhd, (which I can make), to handle the “0x”.
Regards,
Ian
From: Anders Ryd ***@***.***>
Date: Tuesday, 5 October 2021 at 16:02
To: cms-L1TK/firmware-hls ***@***.***>
Cc: Tomalin, Ian (STFC,RAL,PPD) ***@***.***>
Subject: Re: [cms-L1TK/firmware-hls] Test vectors for reduced config (#205)
Ian,
I finally got back to looking at this. It seems that the use of a leading ‘0x’ is not used consistently at all. Seems almost 50/50 (but I have not check all memories). My proposal would be to consistently add the ‘0x’ in all file we write.
Thoughts/suggestions?
-Anders
Anders Ryd
***@***.******@***.***>
On Oct 4, 2021, at 1:09 PM, Ian Tomalin ***@***.******@***.***>> wrote:
There are data format inconsistencies for the InputRouter test data. e.g. Comparing the first column of MemPrints/InputStubs/InputStubs_IL_L1PHID_PS10G_1_A_04.dat with that in either "reduced_config_pr" or "master", this new PR has values such as "0x02", whereas the older files have instead "02".
It was requested in CMSSW issue cms-L1TK/cmssw#85<cms-L1TK/cmssw#85> to add "0x" to this column, so I assume the new test data was added after doing this. (Which branch of our L1Trk CMSSW code was used to do this?). However, FileReaderFIFO.vhd will need a small change to accommodate the extra "0x", so we should make this change for both reduced & non-reduced test data at the same time.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#205 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTIE7JIMN26L2ZECROLUFHNVFANCNFSM5E3WV7CQ>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
This email and any attachments are intended solely for the use of the named recipients. If you are not the intended recipient you must not use, disclose, copy or distribute this email or any of its attachments and should notify the sender immediately and delete this email from your system. UK Research and Innovation (UKRI) has taken every reasonable precaution to minimise risk of this email or any attachments containing viruses or malware but the recipient should carry out its own virus and malware checks before opening the attachments. UKRI does not accept any liability for any losses or damages which the recipient may sustain due to presence of any viruses.
|
aehart
force-pushed
the
reduced_config_test_vectors
branch
from
42fd3b4 to
64781fe
Compare
|
@trholmes The tarballs used here are just those that you produced, but repackaged in the format emData/download.sh expects. I neglected to include the info text files before, but I have since added them back and added a comment with the commit hash to emData/download.sh. @tomalin @aryd For the sake of my sanity, I would propose not trying to do a FW sync with this PR. Indeed, I'd like a few pending changes to make their way into the master branch before doing that. In the meantime, I've introduced an ugly hack to make the input stub test-vectors for the reduced config consistent with the other sets of test vectors: firmware-hls/emData/download.sh Lines 258 to 261 in d4fabfa We can simply undo the hacks here and in the FileReader after the next FW sync. |
Agreed. |
aehart
force-pushed
the
reduced_config_test_vectors
branch
from
d4fabfa to
83a9519
Compare
This PR adds the test vectors for the reduced config that were generated by @trholmes to emData/download.sh.
The tarballs are unpacked as LUTsReduced/ and MemPrintsReduced/ in the emData/ directory. Then for each processing module specified in download.sh, links to the reduced config test vectors are provided in a subdirectory of the usual test vector directory; e.g., the full config test vectors for TC_L1L2F are in emData/TC/TC_L1L2F/, and those for the reduced config are in emData/TC/TC_L1L2F/ReducedConfig/.
I also moved all the C++ top files to a dedicated TopFunctions/ directory with those for the reduced config in TopFunctions/ReducedConfig/. This makes it relatively simple to switch between the full config and the reduced config when running the C-simulation or C-synthesis.
Finally, I created dedicated Tcl scripts for exporting the IPs needed for each of the existing subchains. This is in preparation for adding the summer chain (and subchains thereof), when we will need different IPs for different chains. And in general, it is cleaner if the chains are independent of the Tcl scripts in project/.