Fixes to produce the combined module MemPrint and LUT files for a red…

[aryd]

…uced project

PR description:

This PR has two fixes to produced the LUTs and MemPrint tar files for the reduced combined module configuration. There is a minor fix in VMRouterCM to protect agains not writing the AllStubs memory. (In the reduced configuration this memory is not written for L1 and L2 where the seeing is done.) This also write out all LUT files in the .dat format, i.e. HEX numbers. This simplifies the reading of the files in hdl.

PR validation:

Files were generated and tested by running the reduced combined module chain in HLS.

if this PR is a backport please specify the original PR and why you need to backport that PR:

N/A

Before submitting your pull requests, make sure you followed this checklist:

• verify that the PR is really intended for the chosen branch
• verify that changes follow CMS Naming, Coding, And Style Rules
• verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

[tomalin]

@aryd this fails CI as you didn't run "scram b -j8 code-format" (and check by eye that the format changes look sensible).

[aryd]

Done. I have pushed the updated code now.

-Anders

Anders Ryd @.**@.>

On Aug 15, 2022, at 5:11 PM, Ian Tomalin @.**@.>> wrote:

@arydhttps://github.com/aryd this fails CI as you didn't run "scram b -j8 code-format" (and check by eye that the format changes look sensible).

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1215136528, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTLNZY5H42UNJBI7JFTVZJM3HANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

Please add comment to top of TrackletLUT.h explaining what this class is for.

[tomalin]

Please add a comment describing any function in TrackletLUT.h, whose purpose is not obvious from its name.

[tomalin]

Eliminate hard-wired constants or assign to named constants: 1) Can "12" in https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L16 be replaced by N_SEED from Settings.h? 2) 1.5 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L103 . 3) 10 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L544 4) 52 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L676 5) 0.18 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L867

[tomalin]

@aryd this PR seems to have stalled? I'm waiting for my comments to be addressed before approving it.

[tomalin]

Note from L1TrkAlgo meeting. Anders believes many of the constants were in the code before this PR. He will fix them, but not his top priority, so may take a while. The PR cant be merged until then.

[aryd]

Please add comment to top of TrackletLUT.h explaining what this class is for.

Done

[aryd]

Please add comment to top of TrackletLUT.h explaining what this class is for.

Done

[aryd]

Please add a comment describing any function in TrackletLUT.h, whose purpose is not obvious from its name.

Done

[aryd]

Eliminate hard-wired constants or assign to named constants:

1. Can "12" in https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L16 be replaced by N_SEED from Settings.h?
2. 1.5 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L103 .
3. 10 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L544
4. 52 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L676
5. 0.18 here https://github.com/cms-L1TK/cmssw/blob/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src/TrackletLUT.cc#L867

Have been addressed

[tomalin]

@aryd this needs rebasing to resolve the git conflicts mentioned above.

[tomalin]

@aryd The conflicts with TrackletLUT.cc are not trivial. They arise as your PR #182 and @bryates PR (now merged) https://github.com/cms-L1TK/cmssw/pull/185 both made significant changes to this code.

[aryd]

Ian,

I think I have completed the merge. The code is running and the results seems OK. Let me know if you see any issues.

-Anders

Anders Ryd @.**@.>

On Nov 7, 2022, at 5:47 AM, Ian Tomalin @.**@.>> wrote:

@arydhttps://github.com/aryd The conflicts with TrackletLUT.cchttp://TrackletLUT.cc are not trivial. They arise as your PR #182https://github.com/cms-L1TK/cmssw/pull/182 and @bryateshttps://github.com/bryates PR (now merged) #185https://github.com/cms-L1TK/cmssw/pull/185 both made significant changes to this code.

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1305421751, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTJQOS22KW4LQDVK6OLWHDM3PANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

@bryates Please take a look through class TrackletLUT in Ander's branch of PR 182 https://github.com/cms-L1TK/cmssw/tree/ReducedCombinedModuleFixes , and check that he hasn't accidentally deleted any changes that you made, which remain important.

[tomalin]

@aryd This git PR is still complaining "This branch has conflicts that must be resolved". In addition https://github.com/cms-L1TK/cmssw/tree/ReducedCombinedModuleFixes/L1Trigger/TrackFindingTracklet/src says that TrackletLUT.cc in your branch has not been changed for 12 days. Therefore, if you attempted to fix it 1 hour ago, I think your changed was not pushed.

[bryates]

@bryates Please take a look through class TrackletLUT in Ander's branch of PR 182 https://github.com/cms-L1TK/cmssw/tree/ReducedCombinedModuleFixes , and check that he hasn't accidentally deleted any changes that you made, which remain important.

Looks like all the changes are just comments, and some variables to replace constants. I'm happy with these changes.

[tomalin]

@bryates Please take a look through class TrackletLUT in Ander's branch of PR 182 https://github.com/cms-L1TK/cmssw/tree/ReducedCombinedModuleFixes , and check that he hasn't accidentally deleted any changes that you made, which remain important.

Looks like all the changes are just comments, and some variables to replace constants. I'm happy with these changes.

Hi @bryates Sorry, I think you must wait until Anders has eliminated the git conflict reported in this PR before checking what he's done to your code ...

[aryd]

I think I have resolved the git conflicts? No?

-Anders

Anders Ryd @.**@.>

On Nov 7, 2022, at 11:29 AM, Ian Tomalin @.**@.>> wrote:

@bryateshttps://github.com/bryates Please take a look through class TrackletLUT in Ander's branch of PR 182 https://github.com/cms-L1TK/cmssw/tree/ReducedCombinedModuleFixes , and check that he hasn't accidentally deleted any changes that you made, which remain important.

Looks like all the changes are just comments, and some variables to replace constants. I'm happy with these changes.

Hi @bryateshttps://github.com/bryates Sorry, I think you must wait until Anders has eliminated the git conflict reported in this PR before checking what he's done to your code ...

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1305866926, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTLUB4PSIS57DIIYYD3WHEU5HANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

This branch has conflicts that must be resolved

Hi Anders, I'm afraid the git conflicts are still present. Search for the sentence "This branch has conflicts that must be resolved" in https://github.com/cms-L1TK/cmssw/pull/182 .

[aryd]

Ian, I followed the instructions on the wiki page to resolve the conflicts. I think this went fine. However, I get this message when I try to push the changes:

@. test]$ git push cms-L1TK ReducedCombinedModuleFixes Enter passphrase for key '/afs/cern.ch/user/a/aryd/.ssh/id_rsa':http://cern.ch/user/a/aryd/.ssh/id_rsa': To @*.**@*.>:cms-L1TK/cmssw.git ! [rejected] ReducedCombinedModuleFixes -> ReducedCombinedModuleFixes (non-fast-forward) error: failed to push some refs to @*.**@*.***>:cms-L1TK/cmssw.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

I can make a pull - but I really don’t understand why this would be needed. There has not been any changes in the remote for 13 days and I did the merge yesterday. Is this some side effect of doing the conflict resolving?

-Anders

Anders Ryd @.**@.>

On Nov 7, 2022, at 10:52 AM, Ian Tomalin @.**@.>> wrote:

This branch has conflicts that must be resolved

Hi Anders, I'm afraid the git conflicts are still present. Search for the sentence "This branch has conflicts that must be resolved" in #182https://github.com/cms-L1TK/cmssw/pull/182 .

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1305899031, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTPU423T7MZ4VAUDRDDWHEXUFANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

Ian, I followed the instructions on the wiki page to resolve the conflicts. I think this went fine. However, I get this message when I try to push the changes: @. test]$ git push cms-L1TK ReducedCombinedModuleFixes Enter passphrase for key '/afs/cern.ch/user/a/aryd/.ssh/id_rsa':http://cern.ch/user/a/aryd/.ssh/id_rsa': To @*.**@*.>:cms-L1TK/cmssw.git ! [rejected] ReducedCombinedModuleFixes -> ReducedCombinedModuleFixes (non-fast-forward) error: failed to push some refs to @*.**@*.>:cms-L1TK/cmssw.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. I can make a pull - but I really don’t understand why this would be needed. There has not been any changes in the remote for 13 days and I did the merge yesterday. Is this some side effect of doing the conflict resolving? -Anders Anders Ryd @*.**@*.> On Nov 7, 2022, at 10:52 AM, Ian Tomalin @*.**@*.>> wrote: This branch has conflicts that must be resolved Hi Anders, I'm afraid the git conflicts are still present. Search for the sentence "This branch has conflicts that must be resolved" in #182<#182> . — Reply to this email directly, view it on GitHub<#182 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTPU423T7MZ4VAUDRDDWHEXUFANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.>

Hi @aryd , after doing a rebase, you must use "git push -f cms-L1TK yourBranchName". This is documented in https://twiki.cern.ch/twiki/bin/viewauth/CMS/L1TrackSoftware , if you search for section "https://twiki.cern.ch/twiki/bin/viewauth/CMS/L1TrackSoftware" and then for text "You need to do "git push -f ..." when done.

[aryd]

OK I made a backup and went through the git pull etc. Now I think this is pushed. (But I find the way git does this super confusing as I when I did the git pull it again had to do a merge of the changes in TrackletLUT.{cc,h} this time it did it automatically.)

-Anders

Anders Ryd @.**@.>

On Nov 8, 2022, at 6:42 AM, Anders Ryd @.**@.>> wrote:

Ian, I followed the instructions on the wiki page to resolve the conflicts. I think this went fine. However, I get this message when I try to push the changes:

@. test]$ git push cms-L1TK ReducedCombinedModuleFixes Enter passphrase for key '/afs/cern.ch/user/a/aryd/.ssh/id_rsa':http://cern.ch/user/a/aryd/.ssh/id_rsa': To @*.**@*.>:cms-L1TK/cmssw.git ! [rejected] ReducedCombinedModuleFixes -> ReducedCombinedModuleFixes (non-fast-forward) error: failed to push some refs to @*.**@*.***>:cms-L1TK/cmssw.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Merge the remote changes (e.g. 'git pull') hint: before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

I can make a pull - but I really don’t understand why this would be needed. There has not been any changes in the remote for 13 days and I did the merge yesterday. Is this some side effect of doing the conflict resolving?

-Anders

Anders Ryd @.**@.>

On Nov 7, 2022, at 10:52 AM, Ian Tomalin @.**@.>> wrote:

This branch has conflicts that must be resolved

Hi Anders, I'm afraid the git conflicts are still present. Search for the sentence "This branch has conflicts that must be resolved" in #182https://github.com/cms-L1TK/cmssw/pull/182 .

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1305899031, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTPU423T7MZ4VAUDRDDWHEXUFANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

Hi @aryd, I fear your rebase went wrong. https://github.com/cms-L1TK/cmssw/pull/182 says you have changed a total of 46 files, whereas only TrackletLUT was in conflict, and you had only changed a few files before the rebase.

I suggest you use my recipe below, to remove your bad rebase, starting in an empty CMSSW area:

If you made a mistake and added a bad commit the end of your branch and push it to it, then:

1) Checkout your branch as usual 2) git git reset --hard HEAD~ (remove last commit) 3) git push -f

[aryd]

I think the rebase went fine. I resolved the issued in TrackletLUT.{cc,h}. The code built and ran fine. The results looked as expected.

I think that were things must have gone wrong is in the ‘git pull’ step.

After merging and checking that the code worked I tried to push, but was told I needed to do ‘git pull’ I tried this, but got an error that no branch was specified so I tried

git pull ReducedCombinedModuleFixes

Then it complains that I did not have a remote so I tried:

git pull cms-L1TK ReducedCombinedModuleFixes

This merged the code, but the automatic merging seemed to have worked.

Then I did , git add, git commit, git push

But I now see that this did indeed create merge conflicts. I resolved these - this involved making again the same set of edits as I did 2 or 3 times during the rebaselining. There must be something wrong with the process here…

I tried to compile and run the code and it seems fine.

I then did the usual git add, git commit, and git push

I think this should now be OK.

I also noticed that during the git commit I got a new message:

@.*** TrackFindingTracklet]$ git commit -m "Fix the same merge conflicts yet another time" Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping.

But I presume this is just informational.

-Anders

Anders Ryd @.**@.>

On Nov 8, 2022, at 12:11 PM, Ian Tomalin @.**@.>> wrote:

Hi @arydhttps://github.com/aryd, I fear your rebase went wrong. #182https://github.com/cms-L1TK/cmssw/pull/182 says you have changed a total of 46 files, whereas only TrackletLUT was in conflict, and you had only changed a few files before the rebase.

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1307636181, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTJX3ODLSGRWMSQCYOTWHKJT3ANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

Hi @aryd ,

I suspect that you were doing fine until you reached the step "After merging and checking that the code worked I tried to push, but was told I needed to do ‘git pull’".

This git message is very misleading (as git messages often are). The true reason your git push failed is that you needed to do "git push -f" (always required after rebase). I suspect that the "git pull cms-L1TK ReducedCombinedModuleFixes" messed things up, and explains why this PR now has so many changed files.

The git CI fails complaining that TrackletLUT is still in conflict https://gitlab.cern.ch/cms-l1tk/cmssw_CI/-/pipelines/4734209 .

An illustration of the issue is that looking at the "Files changed" for this PR, one sees that it adds "namespace tt {class Setup}" to line 17 of Settings.h https://github.com/cms-L1TK/cmssw/pull/182/files#diff-a60861ca738c293df9cfb82e85a7dec56e53e508645e3b3a361be1f9dc21c041 , despite the fact that these lines were already present in our default development branch https://github.com/cms-L1TK/cmssw/blob/L1TK-dev-12_5_0_pre2/L1Trigger/TrackFindingTracklet/interface/Settings.h#L17 .

[aryd]

I have a tar file after the rebase, but before I did the ‘git pull’. Do you think I can just do a “git push -f” from what I saved?

-Anders

Anders Ryd @.**@.>

On Nov 8, 2022, at 2:20 PM, Ian Tomalin @.**@.>> wrote:

Hi @arydhttps://github.com/aryd ,

I suspect that you were doing fine until you reached the step "After merging and checking that the code worked I tried to push, but was told I needed to do ‘git pull’".

This git message is very misleading (as git messages often are). The true reason your git push failed is that you needed to do "git push -f" (always required after rebase). I suspect that the "git pull cms-L1TK ReducedCombinedModuleFixes" messed things up, and explains why this PR now has so many changed files.

The git CI fails complaining that TrackletLUT is still in conflict https://gitlab.cern.ch/cms-l1tk/cmssw_CI/-/pipelines/4734209 .

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1307780569, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTKAEOQFKV2VXY2J5GDWHKYYNANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

I have a tar file after the rebase, but before I did the ‘git pull’. Do you think I can just do a “git push -f” from what I saved? -Anders Anders Ryd @.**@.> On Nov 8, 2022, at 2:20 PM, Ian Tomalin @.**@.>> wrote: Hi @arydhttps://github.com/aryd , I suspect that you were doing fine until you reached the step "After merging and checking that the code worked I tried to push, but was told I needed to do ‘git pull’". This git message is very misleading (as git messages often are). The true reason your git push failed is that you needed to do "git push -f" (always required after rebase). I suspect that the "git pull cms-L1TK ReducedCombinedModuleFixes" messed things up, and explains why this PR now has so many changed files. The git CI fails complaining that TrackletLUT is still in conflict https://gitlab.cern.ch/cms-l1tk/cmssw_CI/-/pipelines/4734209 . — Reply to this email directly, view it on GitHub<#182 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTKAEOQFKV2VXY2J5GDWHKYYNANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

Hi @aryd, that might work. It's worth trying. If it fails, then if you can give me the versions of TrackletLUT.cc and .h that have all conflicts resolved, I can try making a PR with these myself (if you like).

[aryd]

Ian,

I do see that the “git pull -f” was indeed in the instructions. But I followed the advice from git…

My tar file was made only for the TrackFindingTracklet directly and not the ‘root’ of the git area so without doing something more complicated I can not just push from this area.

If you can take the files from here:

~aryd/public/TrackletLUT.*

And make a PR it would be great.

-Anders

Anders Ryd @.**@.>

On Nov 8, 2022, at 2:36 PM, Ian Tomalin @.**@.>> wrote:

I have a tar file after the rebase, but before I did the ‘git pull’. Do you think I can just do a “git push -f” from what I saved? -Anders Anders Ryd @.@.> On Nov 8, 2022, at 2:20 PM, Ian Tomalin @.@.>> wrote: Hi @arydhttps://github.com/arydhttps://github.com/aryd , I suspect that you were doing fine until you reached the step "After merging and checking that the code worked I tried to push, but was told I needed to do ‘git pull’". This git message is very misleading (as git messages often are). The true reason your git push failed is that you needed to do "git push -f" (always required after rebase). I suspect that the "git pull cms-L1TK ReducedCombinedModuleFixes" messed things up, and explains why this PR now has so many changed files. The git CI fails complaining that TrackletLUT is still in conflict https://gitlab.cern.ch/cms-l1tk/cmssw_CI/-/pipelines/4734209 . — Reply to this email directly, view it on GitHub<#182 (comment)https://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1307780569>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTKAEOQFKV2VXY2J5GDWHKYYNANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

Hi @arydhttps://github.com/aryd, that might work. It's worth trying. If it fails, then if you can give me the versions of TrackletLUT.cchttp://TrackletLUT.cc and .h that have all conflicts resolved, I can try making a PR with these myself (if you like).

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/cmssw/pull/182#issuecomment-1307795714, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTNWN6T7SRRBZQIGGODWHK2VHANCNFSM552PNG7A. You are receiving this because you were mentioned.Message ID: @.***>

[tomalin]

Conclusion: as we're in a git mess here, I've created a fresh PR with Anders's versions of TrackletLUT.cc and .h at https://github.com/cms-L1TK/cmssw/pull/189 .