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

TRestHits: error in gaussian fit is now defined by the user #428

Merged
merged 4 commits into from
May 28, 2023

Conversation

cmargalejo
Copy link
Member

@cmargalejo cmargalejo commented May 24, 2023

cmargalejo Ok: 19

This PR is required by PR rest-for-physics/detectorlib#82
The hard coded numbers are now arguments of the functions.

@jgalan
Copy link
Member

jgalan commented May 24, 2023

It seems that doHitsCorrection could be a variable that is also controlled by argument? Actually it seems hardcoded to be true.

Therefore, a potential prototyping is:

Double_t TRestHits::GetGaussSigmaZ(Bool_t doHitsCorrection = true, Double_t error = 150)

@jgalan
Copy link
Member

jgalan commented May 24, 2023

I don't see why we need two counters here

for (unsigned int n = 0; n < GetNumberOfHits(); k++, n++) {`

Both take always the same value inside the loop, so n++ would be enough?

@cmargalejo
Copy link
Member Author

It seems that doHitsCorrection could be a variable that is also controlled by argument? Actually it seems hardcoded to be true.

Therefore, a potential prototyping is:

Double_t TRestHits::GetGaussSigmaZ(Bool_t doHitsCorrection = true, Double_t error = 150)

In the beginning I thought about leaving the option to the user, mainly thinking about data in which all the channels are saved. But then I found that it is usually better to just use it always. However, I left it there as a hint in case at some point we decide it should be controled by the user, or even if we want to activate it only if the number of hits is smaller than some number, e.g., like in the comment in line 778: // bool doHitCorrection = nHits <= 18; //in case we want to apply it only to the smaller events.

For now I think it is fine as it is, but I also don't mind changing it if you think it's better.

@jgalan
Copy link
Member

jgalan commented May 25, 2023

It seems that doHitsCorrection could be a variable that is also controlled by argument? Actually it seems hardcoded to be true.
Therefore, a potential prototyping is:

Double_t TRestHits::GetGaussSigmaZ(Bool_t doHitsCorrection = true, Double_t error = 150)

In the beginning I thought about leaving the option to the user, mainly thinking about data in which all the channels are saved. But then I found that it is usually better to just use it always. However, I left it there as a hint in case at some point we decide it should be controled by the user, or even if we want to activate it only if the number of hits is smaller than some number, e.g., like in the comment in line 778: // bool doHitCorrection = nHits <= 18; //in case we want to apply it only to the smaller events.

For now I think it is fine as it is, but I also don't mind changing it if you think it's better.

Perhaps then what you want to pass by argument is the number of hits from which you consider the event has few hits, and correction should be applied.

Double_t TRestHits::GetGaussSigmaZ(Double_t error = 150, Int_t nHits = 1000000)

The correction applies to events with hits below nHits. So if you want, it is possible to add a new parameter at the process that defines for which event size we start to do the correction. The default nHits=1000000 is that correction applies to any event. You could make 1000000 the biggest int number.

@cmargalejo
Copy link
Member Author

cmargalejo commented May 25, 2023

I don't see why we need two counters here

for (unsigned int n = 0; n < GetNumberOfHits(); k++, n++) {`

Both take always the same value inside the loop, so n++ would be enough?

No, they don't take the same value. n starts from 0 and k comes from:

if (doHitCorrection) {
            nAdd = 2;
        }
Int_t nElems = nHits + nAdd;
vector<Double_t> x(nElems), y(nElems), ex(nElems), ey(nElems);
Int_t k = nAdd / 2;

where nAdd is the number of hits that we artificially add to the event.

@cmargalejo cmargalejo requested review from a team May 26, 2023 07:19
@cmargalejo cmargalejo merged commit fb74ef4 into master May 28, 2023
62 checks passed
@cmargalejo cmargalejo deleted the cris_hitsGaussError branch May 28, 2023 16:48
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.

None yet

3 participants