Open EinarElen opened 2 years ago
I've been looking around a bit for these things, @cmantill is a negative TOA value ever reasonable? If not, I might have an idea of where to look into
It shouldn't be, but is possible that the TOA correction is actually causing this? Can you check if it is positive before this line? https://github.com/LDMX-Software/Hcal/blob/trunk/src/Hcal/HcalRecProducer.cxx#L103
I'll mess around and see what I can find!
I went through one call to getTOA that yields a negative TOA. The loop variables unfold as seen in the end of this comment (the counter is the sample number).
This then gives the TOA as
double toa = (maxSample - toaSample) * clock_cycle_ - toaRelStartBX;
// toa = (3 - 3) * 25 - 8.935 = -8.935
// time w.r.t to the SOI
toa += ((int)iSOI - maxSample) * clock_cycle_;
// toa = -8.935 + (3 - 3) * 25 = -8.935
It shouldn't have been possible that it would have been caused by this update 0d1e4b437909f3c1bfee220b52f065bf1f8466da but I checked to confirm. Issue is still there.
Ok the energy is just a factor 2 larger than before. I'm guessing this is because the voltage is defined as
// set voltage as the sum of both bars
voltage = (voltage_posend + voltage_negend);
and the energy is given by
double num_mips_equivalent = voltage / voltage_per_mip_;
double energy_deposited = num_mips_equivalent * mip_energy_ ;
Is this double counting intended? If so, I can just deal with it in my analysis
hi @EinarElen, looking at this on more detail, we made this change of a factor of 2 because of issue 16. I think the num_mips_equivalent
should stay as is since we want the total number of PEs saved as the sum, while we could introduce the facto of 2 in the energy calculation:
double energy_deposited = num_mips_equivalent * mip_energy_ / 2;
Or, we can leave it for the analysis. Do you have a strong opinion?
That would work for me, but wouldn't that create issues for single ended readout which would then get half the energy?
Here's a plot showing the position issue. It shows results for 10k 2 GeV pi- events with the prototype geometry. The histogram contains all hits that have a position along the bar outside of the bounds of the detector
Ok my comments before were a bit confused (or maybe I'm confused now), but I guess that getTOA
returns a relative time so that time being negative in and of itself wouldn't be an issue necessarily. However, the problem still remains. And it isn't caused (at least note solely) by the correction step.
For example, I have an event where
fabs(TOA_posend - TOA_negend) = 80
(with correction)
fabs(TOA_posend_copy - TOA_negend_copy) = 78
(without)
When multiplied with v
, this gives you a position around 8000. Here, TOA_posend = -53
, TOA_negend = 27
, TOA_posend_copy = 75
, and TOA_negend_copy = -3
. So the correction makes a difference but its not obvious here that it is working correctly with bad input.
I'm not sure how to move forward with this on my own, @cmantill do you have any suggestions?
Strange Hcal Reconstruction output
Describe the bug
There seems to be two strange parts of the Hcal reconstruction output (at least with the prototype geometry).
Some hits have positions that are wildly outside of the geometry
This seems to happen when we have one of the TOA values (
TOA_posend
orTOA_negend
) be negative and the other positive. I suspect that negative TOA values aren’t good in the first place, but I’m not sure.The distribution of the reconstructed energy is significantly different from the distribution of energy per bar from simulated hits. Specifically, the total reconstructed energy is significantly larger than the total simulated energy
I haven’t looked into this much but wanted to get the bug report up so I’ll update the issue here. I’m keeping them together because I suspect that they might be related. If they turn out not to be, ill make a separate issue for that.
To reproduce
I don’t think that this should be a problem that is specific to the prototype geometry, unless we have some part of the digitization code that relies on particularities of the regular detector. I haven’t tested with the full detector yet but in principle, you’d just have to change the corresponding line in the configuration file below.
Fastest way to see it is to just run a simulation with the configuration file below, and then just inspecting it either manually or with a script like below.
this should output
With GDB inside of the container you can explore it as follows, which lets us see that the large positions comes from the TOA values. I’m setting three breakpoints, you only need the first one. The second and third will break whenever one (but not two) of the TOA values is negative.
Configuration file