evaluate TGraphError instead of TF1

AndrewLevin commented 4 years ago

Since we take DAC scans by scanning each DAC value 100 times, we have fine-grained and smooth enough data to just interpolate from the data, instead of evaluating a 5th order polynomial, which sometimes does not fit too well.

Description

The TGraphError has a built-in Eval function which performs linear interpolation from two nearby points, so I have used that.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context

This change was proposed and discussed briefly here: https://github.com/cms-gem-daq-project/gem-plotting-tools/pull/265. As discussed there, we are keeping the fifth order polynomial to flag bad VFATs.

How Has This Been Tested?

Yes, I have tested this on data from GE11-X-S-FIT-0004 and found very small changes in the nominal DAC values, as expected. For example:

< 4 17
< 5 24
< 6 23
---
> 4 16
> 5 23
> 6 24
13,14c13,14
< 12    27
< 13    18
---
> 12    28
> 13    19
20c20
< 19    23
---
> 19    22
23c23
< 22    22
---
> 22    21

Screenshots (if appropriate):

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

lpetre-ulb commented 4 years ago

I still agree that, considering the not always very good fifth order polynomial fits, this method is probably the way to go.

However, how do we plan to manage the case where the DAC scan is "strange": http://cmsonline.cern.ch/cms-elog/1095298 ? This might be impacting a single VFAT of the whole GE1/1 sub-detector...

jsturdy commented 4 years ago

If I understand correctly, the proposed change is to evaluate from the TGraphErrors, but to still do the fit (for sanity checking purposes and such)?

Regardless, @lpetre-ulb raises an important point, I don't think we want to blindly accept the interpolation (nor the fit evaluation, actually) in cases where the fit itself does strange things, so what is the (current and new) behaviour in the case where

fitted value would be out of range
interpolated value would not lie on a linear fit connecting most points

AndrewLevin commented 4 years ago

If I understand correctly, the proposed change is to evaluate from the TGraphErrors, but to still do the fit (for sanity checking purposes and such)?

Yes.

Regardless, @lpetre-ulb raises an important point, I don't think we want to blindly accept the interpolation (nor the fit evaluation, actually) in cases where the fit itself does strange things, so what is the (current and new) behaviour in the case where

fitted value would be out of range

The out of range values are handled the same way as when the fit is evaluated.

interpolated value would not lie on a linear fit connecting most points

This happens only in the isolated cases that we should catch at QC7. For the VFAT already installed at point 5 that has this problem, since we basically don't understand what causes the problem, I think we basically don't know whether using the fit is better compared to just interpolating nearby points. It is possible to use a more advanced spline interpolation, but I am not sure if that is worth it.

cms-gem-daq-project / gem-plotting-tools