pixie16 / paass

Pixie Acquisition and Analysis Software Suite
https://pixie16.github.io/paassdoc/
GNU General Public License v3.0
10 stars 29 forks source link

Issue264: Readded QDC compression to the VANDLE histograms #265

Closed tking53 closed 7 years ago

tking53 commented 7 years ago

This should resolve #264

This was requested during the last experiment (IS632). This was picked off the IS632 branch and cleaned up. We had no issues with this on both the Bill laptop and kqxhc (offline scans).

ksmith0 commented 7 years ago

I will not block this, but without testing with VANDLE data it does not make much sense to approve it. The change in functionality as I understand is only with regard to VANDLE and should not affect PSPMT data (although this is a good data point to have as well).

spaulaus commented 7 years ago

I will not block this, but without testing with VANDLE data it does not make much sense to approve it.

I would block this on the argument that this is added to the VandleProcessor. These histograms are meant to be "Raw" data. This compression is an experiment specific requirement and should not be forced into every situation.

In addition, the addition of a dedicated variable is redundant. The parameter is read from the configuration file, which means that it can be accessed via Globals.

tking53 commented 7 years ago

I agree that its an experiment specific requirement, but that is why it defaults to 1 when its not present in the config. This allows us the flexibility to quickly have it if we need it. We ran the IS632 without a experiment specific processor per Miguel's wishes and we might continue this if possible.

    if (!node.child("QdcCompression").empty())
        globals->SetQdcCompression(
                node.child("QdcCompression").attribute("value").as_double());
    else
        globals->SetQdcCompression(1.0);

@spaulaus I see that the dedicated variable is redundant, but i was being consistent with the multiplicity, the offset and the number of starts which are passed in by the DetectorDriverXmlParser. Is there a reason to reference Globals each time over the dedicated variable other than reducing the memory needed by the extra variable?

spaulaus commented 7 years ago

@spaulaus I see that the dedicated variable is redundant, but i was being consistent with the multiplicity, the offset and the number of starts which are passed in by the DetectorDriverXmlParser. Is there a reason to reference Globals each time over the dedicated variable other than reducing the memory needed by the extra variable?

I don't disagree that you could use the same procedure for the QDC compression. In fact, everything should have been updated so that the qdc compression was moved out of Globals and read in through the VandleProcessor constructor. This would enable different processors to have different compression factors. We should consider a new class (DammPlotManipulator?) that handles these variables without the redundancy involved.