Closed iaraota closed 7 months ago
Attention: 18 lines
in your changes are missing coverage. Please review.
Comparison is base (
a50504f
) 50.17% compared to head (be1489b
) 49.87%. Report is 6 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
gwsumm/plot/builtin.py | 0.00% | 17 Missing :warning: |
gwsumm/tabs/data.py | 0.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@eagoetz here is the example of this MR: https://ldas-jobs.ligo-la.caltech.edu/~iara.ota/summary/day/20231112/test1/
The first plot uses this new class and the second use the standard histogram option with the DMT-SNSL_EFFECTIVE_RANGE_MPC
. Note that the resulting plot are different, as the computed range is not equal.
I think for the BNS range, we should use DMT-SNSL_EFFECTIVE_RANGE_MPC
, but this new class might be interesting in case we decide to make the same plot for other systems.
@iaraota Just so I understand the direction you went with this code... did you explore using _get_range()
but this doesn't work somehow? Maybe I don't fully understand why using this method wouldn't work for your use case. Thanks for helping me understand
@iaraota Just so I understand the direction you went with this code... did you explore using
_get_range()
but this doesn't work somehow? Maybe I don't fully understand why using this method wouldn't work for your use case. Thanks for helping me understand
Thank you for the suggestion. I will implement it on the _get_range()
function. I let you know once it is ready for review again. Thanks!
@eagoetz I updated the get_range()
method as you suggested. Thanks!
Here is the new example: https://ldas-jobs.ligo-la.caltech.edu/~iara.ota/summary/gps/1389301018-1389312018/test_class/
The test_class tab has the new class histogram and the test_range has the histogram of L1:DMT-SNSL_EFFECTIVE_RANGE_MPC
with all possible variations I could think of (see x-label).
@iaraota You should add the new class to gwsumm/plot/__init__.py
@eagoetz I added the new class to the __init__
file. Thank you!
@iaraota Sorry to ask for one more thing on this one; it's just because there is a change in some deeper inherited function. Would you be willing to run this PR using the actual configuration files that we use for L1 or H1? It might help just to confirm something hasn't been modified in a way that wasn't intended. After that, I think we'd be good to go
@eagoetz here is the run for hoft.ini
: https://ldas-jobs.ligo-la.caltech.edu/~iara.ota/summary/day/20240115/lock/range/
This configuration has the following plot types: histogram
, plot-omivron-snr-hisogram
and range-histogram
.
@iaraota Ok, thank you so much for your patience. This looks good!
This pull request introduces the
RangeCumulativeHistogramPlot
class, derived fromRangePlotMixin
. This new class facilitates the creation of cumulative density histograms within a specified range.Additionally, this PR includes the option to enable automatic scaling of the histogram in Matplotlib. This can be achieved by setting
range = None
when defining it as'autoscaling'
. Similarly, the'max'
and'min'
options are also added to scale the histogram based on the data.Important observation
The
RangeCumulativeHistogramPlot
class will be computed from the $h(t)$ channel, therefore it is not the most efficient way to plot the cumulative histogram for the BNS range. It is faster to use theDMT-SNSL_EFFECTIVE_RANGE_MPC
channel and the standardhistogram
plot type to do so.