Skip to content

Updates to Vec_ps_refl and VecPSPlotGenerator #402

Merged
kevScheuer merged 4 commits into
masterfrom
vecpsCodeMaintenance
Jul 20, 2026
Merged

Updates to Vec_ps_refl and VecPSPlotGenerator #402
kevScheuer merged 4 commits into
masterfrom
vecpsCodeMaintenance

Conversation

@edbarriga

Copy link
Copy Markdown
Contributor

The main updates are to Vec_ps_refl and VecPsPlotGenerator. However, there is also a minor change to fit.cc . The changes are:
Vec_ps_refl:
-Added a header that introduces the code and its usage
-The number of arguments in the code is increasing, and using positional arguments as input can get confusing. A new key=value method for parsing arguments is introduced, while keeping the old parsing for backward compatibilityhalld. This might also help with consistency since Vec_ps_moments uses the VecPsPlotGenerator, but the positional arguments might be different.
---The new parsing also "proofreads" your input and tries to catch common mistakes... not taking from experience *whistling innocently
---Among the new arguments, the user can now specify if the omega is decaying to gpi0, which should include the photon's helicity, and whether the barrier factors should be turned off
-The TH1 for the polarization fraction Vs energy histogram now detaches from the TFile. This allows the TFile to be closed
-The code related to reading the pol. info stored in the beam 4-vector is now clearer
-The radiative decay for omega has been implemented
VecPsPlotGenerator:
-Corrected some bugs, such as misnames in error outputs and missing names for histograms in the header file
-Added a similar implementation used in Vec_ps_refl to read the arguments so that it is now compatible with positional and key=value method
----The new parsing also solves some problems with the old way of parsing that assumed specific positioning, making it incompatible with storing the pol. info in the beam 4-vector
fit.cc:
-Updated the output for the fit summary. It still ranks the fits that converged, but also displays the error matrix status, the difference between the lowest likelihood and the other fits, and checks if there are solutions that are close in likelihood. The output of this can be "human readable" or csv

@gluex

gluex commented Jul 8, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_vecpsCodeMaintenance.log
Build report: report_vecpsCodeMaintenance.txt

@mashephe

mashephe commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Since you are making a number of changes, I recommend trying to adhere to the coding conventions concerning variable names that is used in AmpTools. Variables starting with "m_" are class member data. I noticed that these revisions have both class member data that does not start with "m_" and local variables that use "m_". I recommend revising for readability.

@edbarriga

Copy link
Copy Markdown
Contributor Author

Thanks for pointing that out. I have updated the names of the variables to keep the coding conventions consistent

@gluex

gluex commented Jul 8, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_vecpsCodeMaintenance.log
Build report: report_vecpsCodeMaintenance.txt

@mashephe

mashephe commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

my apologies for giving you comments piecemeal...

I'm on to your plotter class now. This implementation is not really efficient because it does the same argument parsing for every event in the data sample. This could be done in the constructor of the class. You can have a map from a string that is a reaction name to your struct that holds the arguments. Then do the parsing in the constructor.

Also, since you've written some nice argument parsing code in the Amplitude class, why not use again in the plotter? You could include the amplitude class header in the plotter and then use the static member function to do the parsing. As long as the amplitude class parses everything you need to plot, then you are fine. Seems better to have one method to parse amplitude arguments and one structure to hold them -- just make the plotter class use it.

@gluex

gluex commented Jul 10, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_vecpsCodeMaintenance.log
Build report: report_vecpsCodeMaintenance.txt

@edbarriga

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback! Doing the parsing in the constructor sped up the plotting. The plotter now also uses the functions from the amplitude class.

Comment thread src/libraries/AMPTOOLS_AMPS/Vec_ps_refl.cc Outdated
Comment thread src/libraries/AMPTOOLS_AMPS/Vec_ps_refl.cc
@gluex

gluex commented Jul 16, 2026

Copy link
Copy Markdown

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

Build log: make_vecpsCodeMaintenance.log
Build report: report_vecpsCodeMaintenance.txt

@kevScheuer
kevScheuer merged commit b1bed1c into master Jul 20, 2026
2 checks passed
@mashephe

Copy link
Copy Markdown
Contributor

The Vec_ps_refl class should have this line somewhere in the header. It is not critical, but if every amplitude in a fit needs just the user variables then the framework will purge four-vectors from memory, which can be helpful.

    // we can calculate everything we need from userVars block so allow
    // the framework to purge the four-vectors
    bool needsUserVarsOnly() const { return true; }

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.

4 participants