Updates to Vec_ps_refl and VecPSPlotGenerator

[edbarriga]

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]

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

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

[mashephe]

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]

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

[gluex]

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

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

[mashephe]

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]

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

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

[edbarriga]

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.

[kevScheuer]

The example arguments appear to be incorrect, unless I am mistaken, I believe they should be:

Suggested change

	// For a 2P m=0 wave from an omega->gpi that belongs to the real part of
	// the amplitude sum,has a negative sign in front of the polarization term, and
	// a photon helicity -1, you would write:
	// you would write:
	// If polarization info stored in the beam P4:
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 1 1 2 -1 -1 omegagpi -1
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 1 1 2 -1 -1 omegagpi gHelicity=-1
	// If polarization angle and fraction are fixed:
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 1 1 2 -1 -1 90 .35 omegagpi -1
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 1 1 2 -1 -1 polFraction=0.35 polAngle=90 omegagpi gHelicity=-1
	// If polarization vs E_gamma from histogram:
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 1 1 2 -1 -1 90 pathToFile.root histName omegagpi -1
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 1 1 2 -1 -1 polAngle=90 polFile=pathToFile.root polHist=histName omegagpi gHelicity=-1
	// For a 2P m=0 wave from an omega->gpi that belongs to the real part of
	// the amplitude sum,has a negative sign in front of the polarization term, and
	// a photon helicity -1, you would write:
	// you would write:
	// If polarization info stored in the beam P4:
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 2 0 1 1 -1 omegagpi -1
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 2 0 1 1 -1 omegagpi gHelicity=-1
	// If polarization angle and fraction are fixed:
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 2 0 1 1 -1 90 .35 omegagpi -1
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 2 0 1 1 -1 polFraction=0.35 polAngle=90 omegagpi gHelicity=-1
	// If polarization vs E_gamma from histogram:
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 2 0 1 1 -1 90 pathToFile.root histName omegagpi -1
	// amplitude reactionName::SumName::WaveName Vec_ps_refl 2 0 1 1 -1 polAngle=90 polFile=pathToFile.root polHist=histName omegagpi gHelicity=-1

[edbarriga]

Ups... I'll fix these lines right away

[kevScheuer]

May be better to change these and other errors to the report( <TYPE>, kModule) AmpTools style for consistency. The only one I'm unsure of is for functions like parseValidatedNumber since those are static and take in a contextual argument, so those might have to remain as-is.

[edbarriga]

I don't know. My understanding is that report() is to log the error message, but what we really want here is to terminate the program if any of the inputs are wrong. And, I don't think we want to use report() followed by assert() because these errors are coming from the config file. I am still not 100% clear on this, but the advice I saw online is to use throw for runtime errors. I could use report() followed by throw std::runtime_error(), but that seems a bit redundant

[mashephe]

If you are going to cause the program to exit, then you can do this a number of ways. Exceptions can, in principle, be caught and then recovered from so this allows more functionality to handle runtime errors. I don't think those details matter in this case. The most important thing is that a clear message for exit/abort is provided to the user. I think this does that. It could also be done with the report methods and then exit(1) or assert(false). The report methods are really designed to let the user (and developer) control the amount of stuff that gets printed to the screen and to have a clear explanation of where the messages are coming from.

[gluex]

Test status for this pull request: SUCCESS

Summary: summary.txt
Logs: results/log

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