Skip to content

Conversation

bquilain
Copy link
Collaborator

-Add goodness of fit for vertex.
-Add direction and energy fitting.

@gpronost
Copy link
Collaborator

Once the PR is merged, would it be possible to push these new commits on the hyperk gitlab too?

@bquilain
Copy link
Collaborator Author

Hi @gpronost . Yes totally. I have finally cured my lack of use of gitlab thanks to fiTQun, I will push the commits on gitlab

@bquilain
Copy link
Collaborator Author

@gpronost Sorry for the question out of the blue: I realized the FitterOutput variables have been changed from array of double/int to vectors. My memory may be wrong, but I think you had done it to adapt to multi trigger. Is it correct?

If yes, I have now some issues to write the output in TTree, I think we need to define pointer on vectors. I will modify the code accordingly, but I wished to check with you if my understanding was good, and if you approved this change?

@gpronost
Copy link
Collaborator

gpronost commented Aug 1, 2025

The fitter output was a simple structure before, but seems much more complex/extended now (and use vector yes).
This is not my doing, I guess it comes from Nicolas?

I don't mind between array or vector, but I'm wondering if we really need such extended output structure. Some of the structure members seem to be debug / or are filled but not used. In addition, the output structure itself seems to be used as a function input parameter. However, the way it's done, I think could be better used as a class member instead (since it should not be affected by multi-threading) (and moved the structure member purely used for internal communication to be class member themselves too).

@bquilain
Copy link
Collaborator Author

bquilain commented Aug 1, 2025

Thank you @gpronost . Yes, I agree that the object FitterOutput has somehow lost its original meaning.

@NicolasMorea , do you think you could re-work the code a bit, to separate variables which are used as inputs of some functions, and some others that are really the fitter output? I will start doing it now

@NicolasMorea
Copy link
Collaborator

NicolasMorea commented Aug 1, 2025

Thank you very much for the feedback.

FitterOutput does contain some extra values that I was planning to remove before the end of the internship. Iwill also do a lot of cleanup on files like Analysis.cc, which has grown significantly during my time here. (I am thinking I could create another struct, like MetaDataOutput, to hold all the non-essential variables such as the true values)

I used vectors wherever possible to support multi-trigger functionality more easily. Also, since everything is of the same type, I can reuse the same functions for different vector variables, which improves code readability (this includes functions in LeafUtility.cc). This approach does require using pointers to store the data in the ROOT file, which was already done this ways for some variables like True_Origin, but not all so I wanted to standardize that.

I passed the fitter output as a function parameter so it's clear which functions can modify it, but I can easily refactor it as a class member if needed. Thank you again for the suggestion.

@bquilain
Copy link
Collaborator Author

bquilain commented Aug 1, 2025

Thank you Nicolas, it is exactly inline with, I believe, Guillaume's comment, and what I had in mind.

I would like to try to have a merged version quickly that we could improved later. Do you mind if I add some subtle modifications in your branch?

@NicolasMorea
Copy link
Collaborator

Ok perfect. And no of course I don't mind, you can modify the code !

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

Successfully merging this pull request may close these issues.

3 participants