Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize reco to allow for a larger variety of plane types #156

Open
jdkio opened this issue Sep 17, 2024 · 3 comments
Open

Generalize reco to allow for a larger variety of plane types #156

jdkio opened this issue Sep 17, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@jdkio
Copy link
Contributor

jdkio commented Sep 17, 2024

I know Chris and Kiyoung want to look at TMS with pure XY planes. And there are a variety of others that people may want to check.

The simplest thing is to start by finding all the locations in the reco code where plane types are mentioned. I think @AsaNehm and @LiamOS are best suited to doing this

The way I see it, the reco code has three types of functions

  1. Functions that work on one plane at a time
  2. Functions that loop over multiple views
  3. Functions that combine information from multiple planes

For the first case, the code should be simplified to only use plane-specific code has needed. For example, GetHoughLine doesn't use X,Y,V, or U, and instead uses GetZ and GetNotZ. Then the function calling GetHoughLine knows how to interpret the resulting slope and intercept information

As an example of type 2, FindTracks does:
HoughCandidatesU = HoughTransform(TrackCandidatesU, 'U');
This is an example of something that could be modified to

for (auto view : views) {
   HoughCandidatesMap[view] = HoughTransform(TrackCandidatesMap[view], view);
}

TrackMatching3D is an example of type 3. It needs some idea of the views. This should be more dependent on TMS_Geom.h given different setups.

There are also specialized variables that are saved per view like HoughCandidatesU above. It might make more sense to modify these to be a map where the key tells us the view. Or we can make specialized objects that know their own view

These modifications are long-term goals. The short term goal should be to tag what would need to be changed and maybe make short, medium, and long term plans

@jdkio jdkio added the enhancement New feature or request label Sep 17, 2024
@AsaNehm
Copy link
Contributor

AsaNehm commented Sep 18, 2024

I talked to Kiyoung about this and we figured out that most changes need to be made in TMS_Reco.cpp actually. I don't really have the time to do this but would be happy to help out in as long as I don't have to do the entire work by myself.
On another note, when I initially expanded the code to be able to take into account the actual plane orientation I chose this way of multiplying the code as it was the easiest for me to do quickly. I admit that there are definitely better ways to do it by adapting the code in an orientation-independed way, but I would consider this an enhancement for the future.
So for now creating a new branch and doing it in the same way of simply repeating functions for a new orientation and adapting them wherever information is combined seems like the fastest solution to me

@AsaNehm
Copy link
Contributor

AsaNehm commented Sep 30, 2024

I looked a bit more into this.
As far as I can see there are no specific functions (anymore) for certain orientations. The one that exist at the moment are necessary. Most functions work on multiple orientations even though not in a pretty way in many cases.
In TMS_Reco.cpp the function FindTracks() has quite some repetitions of code (see lines 430-450 = 451-469 = 470-488 and 567-606 = 608-642 = 644-678). This is definitely not great and I'll try to simplify this a lot.
Also in TMS_Reco.cpp the functions HoughTransform(), RunHough() and BestFirstSearch() work with conditions for different orientations. This helped to avoid simple repetition but is difficult to read. I'll try to simplify this as well, but it requires some restructuring of code and saving stuff at very different places (wherever the function is called will then have to take care of the saving instead of within the called function).
At the moment I don't have a better solution for the third type of function (matching in 3D) but as this requires a very good insight of the detector geometry and knowing the requirements to match tracks, I would only change this at a much later point, when we actually know how the orientation layout is.

@AsaNehm
Copy link
Contributor

AsaNehm commented Oct 1, 2024

I removed the repetition of code in FindTracks(). It compiles, but I haven't actually tested the outcome. Please have a look at the attached branch to this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants