search

icecube / skymap_scanner

A distributed system that performs a likelihood scan of event directions for IceCube real-time alerts using CPU cluster(s) and queue-based message passing.
5 stars 2 forks source link

[minor] update to use prob.v2 splines #257

Closed tianluyuan closed 9 months ago

tianluyuan commented 9 months ago

Use an updated SPICE ftp-v1 timing spline with time knots shifted slightly to correct for the bin width when fitting the CDF

Also adds BadDomsList to the DetectorStatus frame if it doesn't exist. closes #219

dsschult commented 9 months ago

The main thing I dislike about this is that it's a change in results without any sort of name or version change (as shown by the tests failing). I guess we could call this a major release of skymap scanner. Otherwise, calling it MillipedeWilksV2 might be better.

Mostly trying to keep it clear to future people why their results have changed.

tianluyuan commented 9 months ago

That's a good point. Since results can also change with icetray updates, and those updates typically increment a minor/patch (?) skymap-scanner version, I think a major version bump may be preferable here.

Now that icetray 1.9.1 will be up soon, there are also some updates in the BadDOMsList that I would like to get in, and maybe can be added as part of this.

tianluyuan commented 9 months ago

@dsschult are you ok with a minor version bump? looping in @ric-evans here as well

ric-evans commented 9 months ago

This appears to be a bug fix, an important one, but not a fundamental change to the scanner. IOW, the results may be different now, but they were incorrect before, right?

ric-evans commented 9 months ago

The system I've used for versioning is: major = breaking/fundamental change, minor = new feature or very important bug fix, patch = bug fix.

dsschult commented 9 months ago

We could follow the icetray model, which has minor releases for when physics is expected to change. And also only change icetray images on minor releases.

So yes, I'd be okay with a minor release here. Could we add a note somewhere in the README defining release types and what is expected to change for each level?

ric-evans commented 9 months ago

We could follow the icetray model, which has minor releases for when physics is expected to change. And also only change icetray images on minor releases.

I like this model. @tianluyuan could you add a small section to the README? So, "minor" is reserved for physics changes and new features.

tianluyuan commented 9 months ago

We could follow the icetray model, which has minor releases for when physics is expected to change. And also only change icetray images on minor releases.

I like this model. @tianluyuan could you add a small section to the README? So, "minor" is reserved for physics changes and new features.

Added to #258

tianluyuan commented 9 months ago

closes #219

ric-evans commented 9 months ago

closes #219

I think you need to put this in the description for github to link the two