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

Gauss hit zeroes fit #254

Merged
merged 25 commits into from
Jul 20, 2022
Merged

Gauss hit zeroes fit #254

merged 25 commits into from
Jul 20, 2022

Conversation

cmargalejo
Copy link
Member

@cmargalejo cmargalejo commented Jun 17, 2022

cmargalejo Large: 559

This pull request aims to solve some issues that were found with the observables GaussSigmaX, GaussSigmaY and GaussSigmaZ, which are computed by fitting a gaussian to each event.

Some of the events were found to have sigmas that were too large. After a deep study, the hypothesis is that the fit fails especially for small events that have too few hits. Not seeing a hit doens't necesarily mean there is no energy on that channel, but that it is below the threshold. Thus, the proposed solution is to add one hit to the left and one hit to the right, at a value 0+/-error.

The main changes are:
-Adding two hits to each hits event.
-Define the energy of these hits as 0 +/- 70 ADC. This error is defined based on typical for the micromegas detectors.

To illustrate the changes, the image below shows the same event before the changes in the code in the left, and the new fit in the right.

image

@jgalan
Copy link
Member

jgalan commented Jun 20, 2022

Hi @cmargalejo , the problem with the pipeline is surely related to the fact that your branch is not updated with master.

You should do locally:

git checkout master
git pull
git checkout gaussHitZeroesFit
git merge master
git push

So that this PR gets updated

@cmargalejo cmargalejo marked this pull request as ready for review July 15, 2022 17:29
@@ -545,103 +546,183 @@ void TRestHits::GetBoundaries(std::vector<double>& dist, double& max, double& mi
Double_t TRestHits::GetGaussSigmaX() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add a description to this method. Perhaps reusing some text from the PR description. I see this class is not following the standard headers. I will just quickly updated that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description added in 154df75

@jgalan
Copy link
Member

jgalan commented Jul 16, 2022

-Define the energy of these hits as 0 +/- 70 ADC. This error is defined based on typical for the micromegas detectors.

What it happens if you set the new hits on the sides to equal zero? If you add the new hits, then the energy of the event will be affected, or the hits are added just for the gaussian fit, then removed for further processing?

@jgalan
Copy link
Member

jgalan commented Jul 16, 2022

Perhaps it would be interesting to add the image you attached to the PR description to the documentation.

@cmargalejo
Copy link
Member Author

-Define the energy of these hits as 0 +/- 70 ADC. This error is defined based on typical for the micromegas detectors.

What it happens if you set the new hits on the sides to equal zero? If you add the new hits, then the energy of the event will be affected, or the hits are added just for the gaussian fit, then removed for further processing?

Yes, the hits are only added to a TGraph to perform the fit, so they don't go into any further processing. Still, their energy is 0.

@nkx111 nkx111 self-requested a review July 19, 2022 13:16
@cmargalejo cmargalejo merged commit e6ba7f2 into master Jul 20, 2022
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

4 participants