search

gwpy / gwsumm

Gravitational-wave interferometer summary information system
GNU General Public License v3.0
12 stars 23 forks source link

Fix case when data is missing at beginning of vt span #382

Closed dethodav closed 10 months ago

dethodav commented 10 months ago

This PR addresses #381 so that the combined VT calculation is correct in all cases where data from at least one detector is missing.

In cases where data is entirely missing from one detector, this detector also is no longer considered during the combined VT calculation.

This PR can be tested by running the O4a summary command with the production archive file.

dethodav commented 10 months ago

I can't assign anyone so, @iaraota @eagoetz @duncanmmacleod

eagoetz commented 10 months ago

Thanks @dethodav! Can you post links showing the difference in calculated values / plots?

dethodav commented 10 months ago

Here is an example of combined before/after this change:

codecov[bot] commented 10 months ago

Codecov Report

Merging #382 (f0b2f91) into master (f658c15) will decrease coverage by 0.05%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #382      +/-   ##
==========================================
- Coverage   50.25%   50.20%   -0.05%     
==========================================
  Files          60       60              
  Lines        8670     8679       +9     
==========================================
  Hits         4357     4357              
- Misses       4313     4322       +9
Flag Coverage Δ
Linux 50.20% <0.00%> (-0.05%) :arrow_down:
python3.10 50.20% <0.00%> (-0.05%) :arrow_down:
python3.7 49.40% <0.00%> (-0.05%) :arrow_down:
python3.8 50.20% <0.00%> (-0.05%) :arrow_down:
python3.9 50.20% <0.00%> (-0.05%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
gwsumm/plot/range.py 37.33% <0.00%> (-1.62%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

eagoetz commented 10 months ago

@dethodav Naïvely, I would have expected that the combined VT should fall in between H1 and L1, but is this because the VT for Virgo is not being plotted yet it is pulling the combined VT downward? If a detector is not in observing, shouldn't it not be applied to this data? Can you help clarify what is going on? I have seen this behaviour on the O3a epoch page https://ldas-jobs.ligo.caltech.edu/~detchar/summary/1238166018-1253977218/range/ and I suspected it was because of Virgo, though I don't understand why it pulls the combined VT lower since L1 should be so much higher

dethodav commented 10 months ago

@eagoetz The combined range is defined as the range of the second most sensitive detector at that time. Hence, for 2 detectors, the combined VT will always be lower than the individual VTs (since there will be some times when only 1 detector is online)