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

Gain computation with SPE fit and Photo-statistic methods #38

Merged
merged 100 commits into from
Mar 17, 2023

Conversation

guillaumegrolleron
Copy link
Contributor

@guillaumegrolleron guillaumegrolleron commented Feb 5, 2023

There are 2 modules in the NectarGain package :

  • PhotoStat : for gain computation (high and low gain) from a pedestal run, a flat field run and a SPE fit result
  • SPEfit : for gain computation with SPE fit (only high gain).

Gain can be computed on ~1400V data, or nominal runs using results from 1400V fit (there are 2 method to perform the combined fit)

This package is based on the container module, input data used in SPEfit and PhotStat have to be container.

Note : This PR follows the PR #37

guillaume.grolleron added 3 commits February 16, 2023 13:48
-typo bugfix : in b equation (1-p**2) instead of (1-p)**2
-optimisation of multiprocessing (unshared variables)
-photostat : use only fullwaveform sum for extract charge from Ped
 and FF
 -figpath missing in SPE fit computation
@jlenain
Copy link
Collaborator

jlenain commented Feb 21, 2023

Running a couple of tests with the different algorithms. So far, gain_SPEfit_computation.py works perfectly (technically speaking, I am not assessing here the scientific quality of the results). Testing gain_PhotoStat_computation.py and gain_SPEfit_combined_computation.py...

Everything working fine, with photostatistics method or SPE fits for single runs or combined fits.

To me, we're good to merge.

.gitignore Outdated Show resolved Hide resolved
@dkerszberg
Copy link
Contributor

Thank you @jlenain
I just left one minor comment, @guillaumegrolleron can you check it?
@vmarandon if all good for you can you "approve" the PR?

@kosack
Copy link
Collaborator

kosack commented Feb 23, 2023

Does this have any connection to the spefit library used by other cameras in the ASWG repository?
https://gitlab.cta-observatory.org/cta-consortium/aswg/tools/spefit

@jlenain
Copy link
Collaborator

jlenain commented Feb 23, 2023

Does this have any connection to the spefit library used by other cameras in the ASWG repository? https://gitlab.cta-observatory.org/cta-consortium/aswg/tools/spefit

Thanks, @kosack . First, I think nobody here had any idea of the existence of this spefit library, so thanks a lot for pointing it out.

Looking quickly at the description of spefit though, there seems to be the case of the double gaussian fit method, as exposed in https://ui.adsabs.harvard.edu/abs/2019SPIE11119E..1WC/abstract, which is covered here but not there.

@kosack
Copy link
Collaborator

kosack commented Feb 23, 2023

Thanks, yeah, I was just wondering if every camera is doing this separately or if there is some coordination? I would guess the methods should be the same for everyone. I imagine there is little inter-telescope coordination in CTA though!

Copy link
Collaborator

@jlenain jlenain left a comment

Choose a reason for hiding this comment

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

Ready to be merged, in my opinion!

@dkerszberg
Copy link
Contributor

Also fine for me. @vmarandon had one final comment for @guillaumegrolleron can you have a look?

@jlenain
Copy link
Collaborator

jlenain commented Mar 1, 2023

Also fine for me. @vmarandon had one final comment for @guillaumegrolleron can you have a look?

@guillaumegrolleron is on it!

@jlenain
Copy link
Collaborator

jlenain commented Mar 17, 2023

Thanks a lot for the hard work, @guillaumegrolleron ! I am merging this PR now.

@jlenain jlenain merged commit 14ee616 into cta-observatory:master Mar 17, 2023
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

Successfully merging this pull request may close these issues.

5 participants