Implementation of TS clustering and tracking as envisioned in LUT firmware.

[rodwyer100]

This push request takes the validated HLS firmware code and reincorporates it into the ldmx-sw structure. I have included in TrigScintFirmwareProducer a producer which takers clusterproducer_sw and trackproducer_sw (faithful reproductions of the code used to implement tracking in HLS), stages a collection of digis for insertion, and then obtains a collection of tracks for use thereafter. The producer as of yet does not save the collected digis into the track object, nor does it produces clusters, but it does faithfully reproduce the tracks of the existing horizontal geometry. There are two caveats with this:

1. It completely reproduces ldmx reconstruction results with no inefficiency in HLS because HLS does a better job at staging the rather poorly behaves ap_int classes it uses. I had some initial issues getting this code to be 100% efficient (its 98% efficient now) because ap_int behaves irregularly in how its implemented here.
2. This Firmware design was used to implement the original horiztonal 3 pad geometry (ldmx-det-v14) and does not have a LUT that corresponds to vertical bars. If this is desired, one could envision changing trackproducer_hw and testing it, but it will require further work.

• [x] I successfully compiled ldmx-sw with my developments
• [x] I ran my developments and the following shows that they are successful.

  I validated that the producer made the same number of tracks using print out statements like this: 10 10 10 18 18 18 29 29 30 [ ElectronCounter ] 0 : Found 3 electrons (tracks) using input collection TriggerPadTracks_sim I have not validated this code as extensively as the firmware version, so it could presumably make tracks from different clusters but my intuition about how the code works tells me that is very unlikely.

Here is a picture of the track number distribution for both: Track no for original reconstruction: Track no for firmware reconstruction: If any more plots are necessary please lmk

[tvami]

I am adding some includes in /includes/ which are necessary for the primitive non floating point objects that HLS uses to create firmware from C++ files.

How are these files different from what we have in Trigger/HLS_arbitrary_Precision_Types/include? If they are not you should not add them again, but rather reuse them

[rodwyer100]

We should talk about how ap_int is implemented in the ldmx-sw framework. I know TS in the Trigger repository doesn't use it, and I suspect it had something to do with the <> in the fixed point library requiring ap_int to be found as a standard directory (and therefore not being found). While there are a couple repositories in the Ecal Trigger repositories that use it I am not certain how Christian managed to work with the relative paths indicated by the use of quotation marks and still have it built with <>. That doesn't seem to work for me, though Im happy to take suggestions. I have implemented a fix that should work for both of us, and that is that in my local copy I have all of the includes of the HLS objects switched (wherever relevant) from <> to "". The issue is that since these are contained in a submodule we don't have access to, I can push that version which would work for us both. I know Tom worked on dereferening TrigScint and I am wondering if that is called for here, or if there is something better one should do. Otherwise I am not certain how to proceed on that front.

[tvami]

The issue is that since these are contained in a submodule we don't have access to,

I'm confused by this statement. Doesn't this mean you just forgot the "recursive" when cloning in git clone --recursive git@github.com:LDMX-Software/ldmx-sw.git ? It should really be available like that

[tomeichlersmith]

Christian got the relative path includes working by telling CMake where to find the necessary headers.

https://github.com/LDMX-Software/ldmx-sw/blob/77fffe17e87e4237d9aaa106d9c343fc7f805947/Trigger/Algo/CMakeLists.txt#L39

This type of thing would be necessary in TrigScint as well if the ap_* headers are of use. Without a line like this in the CMake, there would be no way for the compiler to figure out where the headers are and thus would emit a unable to find ap_XXX.h style compiler error.

[tomeichlersmith]

I did a quick test and this branch still compiles after the following changes.

git rm -r TrigScint/include/TrigScint/etc

diff --git a/TrigScint/CMakeLists.txt b/TrigScint/CMakeLists.txt
index 2d1a8473..05382878 100644
--- a/TrigScint/CMakeLists.txt
+++ b/TrigScint/CMakeLists.txt
@@ -44,6 +44,7 @@ setup_library(module TrigScint
               dependencies Framework::Framework Recon::Event DetDescr::DetDescr
                            Tools::Tools SimCore::Event
 )
+target_include_directories(TrigScint PUBLIC ../Trigger/HLS_arbitrary_Precision_Types/include)

 setup_python(package_name LDMX/TrigScint)

diff --git a/TrigScint/include/TrigScint/objdef.h b/TrigScint/include/TrigScint/objdef.h
index 230cf112..b9706db9 100755
--- a/TrigScint/include/TrigScint/objdef.h
+++ b/TrigScint/include/TrigScint/objdef.h
@@ -1,7 +1,7 @@
 #ifndef OBJDEF_H
 #define OBJDEF_H

-#include "../../../Trigger/HLS_arbitrary_Precision_Types/include/ap_int.h"
+#include "ap_int.h"
 #define NTIMES 6
 #define NHITS 25
 #define NCLUS 25

[rodwyer100]

I have added every change (smart pointers, general adoption of existing capitalization structure, etc) with the exception of clang formatting. I am trying to get my clang formatter working, but s3df doesnt allow for sudo so I have to git clone it and make it and Im on hour 6 or 7 of that. So hopefully it will all be done tomorrow. Please lmk if the chosen way of aligning with the capitalization structure is sufficient!

[tvami]

@rodwyer100 after we merge https://github.com/LDMX-Software/ldmx-sw/pull/1464 the clang format will automatically be applied if it's too much work for you. (Tho I use s3df too and just format-cpp works really nicely w/o any sudo needs)

[rodwyer100]

The files have been clang formatted. Lmk if you think it needs anything else :)!

[tvami]

The files have been clang formatted. Lmk if you think it needs anything else :)!

The only thing missing from my review is https://github.com/LDMX-Software/ldmx-sw/pull/1461#discussion_r1764339136

[rodwyer100]

Whoopsie just saw your comment. Fixed that change now. Thanks so much!

[tvami]

(not directly connected to this PR, but I'm happy to see the bot is doing the clang update as expected, Rory dont be surprised for this extra commit in your branch)