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

Added scripts for processing PACES data #1008

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MikeBowry
Copy link

Added to GRSIProof/ : a basic script ("BasicPACES") and a more complex version ("PACESAngularCorrelation") for gamma-electron angular correlations. Both include functions for energy non-linearity corrections in PACES. "GRIFFIN_PACES_Angle_Calc" can be used to generate GRIFFIN-PACES angle groups for insertion into the PACESAngularCorrelation header file. Theta and Phi measurements for PACES were also updated to the current values in the GetPosition() function of TPaces.

 a basic version and a more compelx version for gamma-electron angular correlations. Both include functions for energy non-linearity corrections in PACES. GRIFFIN_PACES_Angle_Calc can be used to generate GRIFFIN-PACES angle groups. Theta and Phi measurements for PACES were also updated to the current values in the GetPosition function.
@VinzenzBildstein
Copy link
Member

I see a number of issues in this code:

  • it doesn't adhere to our coding standards, see the wiki or the link that comes up when submitting an issue (e.g. member names starting with f and using camel case)
  • as far as I an see the corrections should only need to be calculated once per detector. This should be done in the CreateHistograms function (or the constructor). The resulting graphs should be stored in a vector or map that uses the detector number as key. That way they are only calculated once and can then be used later w/o re-creating them all the time.
  • also I'm not sure that using hard-coded values for the correction is the way to go, especially not for a completely flat function. Wouldn't it be better to use GValues and read them from a .val file? And why are the value copied from an array into a vector only to access them as an array?
  • in TPaces::GetPosition I would use a loop to access the positions that have been calculated once in a single loop

There might be more issues, but those are the more important ones I've noticed.

@MikeBowry
Copy link
Author

Hi Vinzenz
-I can certainly make some changes so that the code conforms with the standard.
-Yes, recreating and destroying each graph for every hit is not ideal. I'll try adding this to the header file.
-I'm not sure I understand here. Can you explain a little more about the alternative suggested? But yes I probably don't need to change the casting in-between (may have done this because it was easier to add to the TGraph, not sure).
-I realized a little while after submitting this update that the TPaces::GetPosition part can be completely removed as the grouping is defined explicitly in an array. I prefer this method for a small number of angle combinations, rather than calculating on the fly.

@r3dunlop
Copy link
Member

r3dunlop commented May 28, 2018 via email

@VinzenzBildstein
Copy link
Member

@r3dunlop is right, you can just make the corrections a static variable (inside CreateHistograms) or declare them globally (e.g. just above CreateHistograms) and fill them in CreateHistograms.

Right now the corrections you have are the same (zero) irregardless of the energy. So it doesn't make any sense to apply the directions the way they are right now. If you meant this to be just an idea for how the corrections could be applied, I would suggest you use GValues to read them from a .val-file. That way anyone using this can make their own .val file with custom non-linearities and run the standard selector with it. It doesn't make sense for us to write a standard selector for PACES if everyone using it has to first change it. Check the GValue class and how it can be used. I think I used them at one point for DESCANT, so you could look at older GRSISort versions and look at the Descant::CorrectedTimeStamp function to see how that can be done.

@r3dunlop
Copy link
Member

r3dunlop commented May 28, 2018 via email

@MikeBowry
Copy link
Author

This was indeed intended to be a template for applying non-linearity corrections. These corrections are likely to be different for each PACES experiment, so I thought I would just set them to zero for now. It is worth noting that most (if not all) of the experiments using PACES so far have been slightly different (thresholds, change to the MSC table, swapping out detectors etc.). For example, I had to modify the angular correlation script to include only three detectors due to poorer resolution in 2/5 of them, but that may not be the case in the future. So anyway there is something to be said for a separate PACES selector. I can look into using GValues and including the output in Create Histograms.

@r3dunlop
Copy link
Member

r3dunlop commented May 28, 2018 via email

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