add bit into AllStubInner for TP Disks #230
Conversation
|
It looks like the test vectors created from this PR https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gzcauses a mismatch in the MP. @jasonfan393 and @aryd does anything need to be changed in the MP (at least on the emulation side) to handle the changes in |
|
PR 280 successfully ran the CI before being merged and also in my local tests. I’m not sure what the issue might be. I’m in meetings and can not look carefully right now.
-Anders
Anders Ryd
***@***.******@***.***>
On Aug 7, 2023, at 6:16 PM, Brent R. Yates ***@***.******@***.***>> wrote:
It looks like the test vectors created from this PR
https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz
case a mismatch in the MP. @jasonfan393<https://github.com/jasonfan393> and @aryd<https://github.com/aryd> does anything need to be changed in the MP (at least on the emulation side) to handle the changes in Stub.h and Stub.cc<http://Stub.cc>?
—
Reply to this email directly, view it on GitHub<#230 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTPUGEOJBHMIVA7H4T3XUEIGPANCNFSM6AAAAAA2MD77TM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Brent,
I did not read your email carefully - you are asking about the CMSSW PR? but I was confused because:
https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz
this is used in the firmware-hls PR #280. I now see that the MP did fail, but the overall CI passed. I’ll try to look at this later.
-Anders
Anders Ryd
***@***.******@***.***>
On Aug 7, 2023, at 6:16 PM, Brent R. Yates ***@***.******@***.***>> wrote:
It looks like the test vectors created from this PR
https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz
case a mismatch in the MP. @jasonfan393<https://github.com/jasonfan393> and @aryd<https://github.com/aryd> does anything need to be changed in the MP (at least on the emulation side) to handle the changes in Stub.h and Stub.cc<http://Stub.cc>?
—
Reply to this email directly, view it on GitHub<#230 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTPUGEOJBHMIVA7H4T3XUEIGPANCNFSM6AAAAAA2MD77TM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Brent,
my previous emails were a bit confused as I was multitasking… I did not connect that the problem was in the MP… I read it as TP…
I ran the MP standalone and tried all the different Layers and Disks that are in the script_MP.tcl file. There is only one single differences in L2 where the HLS code picks up one extra match. So I don’t think that there is any significant change in the test vectors, but that we somehow had different events that tripped some outstanding difference between the HLS and emulation.
-Anders
Anders Ryd
***@***.******@***.***>
On Aug 7, 2023, at 7:25 PM, Anders Ryd ***@***.******@***.***>> wrote:
Brent,
I did not read your email carefully - you are asking about the CMSSW PR? but I was confused because:
https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz
this is used in the firmware-hls PR #280. I now see that the MP did fail, but the overall CI passed. I’ll try to look at this later.
-Anders
Anders Ryd
***@***.******@***.***>
On Aug 7, 2023, at 6:16 PM, Brent R. Yates ***@***.******@***.***>> wrote:
It looks like the test vectors created from this PR
https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz
case a mismatch in the MP. @jasonfan393<https://github.com/jasonfan393> and @aryd<https://github.com/aryd> does anything need to be changed in the MP (at least on the emulation side) to handle the changes in Stub.h and Stub.cc<http://stub.cc/>?
—
Reply to this email directly, view it on GitHub<#230 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTPUGEOJBHMIVA7H4T3XUEIGPANCNFSM6AAAAAA2MD77TM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
On top of the 1 extra, there are also some mismatches, possibly due to HLS picking up extras that should be overwritten, though I haven't dug into it yet to confirm whether they should be overwritten. |
|
You mean:
Event: 59
FullMatch 0:
index reference computed
0 0x42114918101f6 0x420137b5f81f4 <=== INCONSISTENT
1 0x442130160f1da 0x42114918101f6 <=== INCONSISTENT
2 0x44410609f7007 0x442130160f1da <=== INCONSISTENT
3 0x44493a26051d5 0x44410609f7007 <=== INCONSISTENT
4 0x0 0x44493a26051d5 <=== EXTRA
FullMatch 1:
This is one extra match "0x420137b5f81f4 “ at the start and then the other matches are listed as inconsistent.
-Anders
Anders Ryd
***@***.******@***.***>
On Aug 7, 2023, at 8:24 PM, Brent R. Yates ***@***.******@***.***>> wrote:
Brent, my previous emails were a bit confused as I was multitasking… I did not connect that the problem was in the MP… I read it as TP… I ran the MP standalone and tried all the different Layers and Disks that are in the script_MP.tcl file. There is only one single differences in L2 where the HLS code picks up one extra match. So I don’t think that there is any significant change in the test vectors, but that we somehow had different events that tripped some outstanding difference between the HLS and emulation. -Anders Anders Ryd @.@.> On Aug 7, 2023, at 7:25 PM, Anders Ryd @.@.>> wrote: Brent, I did not read your email carefully - you are asking about the CMSSW PR? but I was confused because: https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz this is used in the firmware-hls PR cms-sw#280<cms-sw#280>. I now see that the MP did fail, but the overall CI passed. I’ll try to look at this later. -Anders Anders Ryd @.@.> On Aug 7, 2023, at 6:16 PM, Brent R. Yates @.@.>> wrote: It looks like the test vectors created from this PR https://cernbox.cern.ch/remote.php/dav/public-files/G0AQPtbx74QMKQh/MemPrints.tar.gz case a mismatch in the MP. @jasonfan393<https://github.com/jasonfan393>https://github.com/jasonfan393 and @aryd<https://github.com/aryd>https://github.com/aryd does anything need to be changed in the MP (at least on the emulation side) to handle the changes in Stub.h and Stub.cc<http://Stub.cc>http://stub.cc/? — Reply to this email directly, view it on GitHub<#230 (comment)<#230 (comment)>>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTPUGEOJBHMIVA7H4T3XUEIGPANCNFSM6AAAAAA2MD77TM. You are receiving this because you were mentioned.Message ID: @.***>
On top of the 1 extra, there are also some mismatches, possibly due to HLS picking up extras that should be overwritten, though I haven't dug into it yet to confirm whether they should be overwritten.
—
Reply to this email directly, view it on GitHub<#230 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABI4LTOVIJRPQEQG7YEK6YLXUEXERANCNFSM6AAAAAA2MD77TM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
as discussed during L1TK meeting on Aug 8, this is good to be merged. |
* add bit to allstubinner needed for TP Disk expansion * formatting * run scram b -j code-format
* add bit to allstubinner needed for TP Disk expansion * formatting * run scram b -j code-format
* add bit to allstubinner needed for TP Disk expansion * formatting * run scram b -j code-format
PR description:
PR records an extra bit into disk allStubInner memories test vectors. This bit is used by the TP in HLS to differentiate the postive and negative disks.
corresponds to firmware_hls PR (280)
this is a copy of closed PR: 229