dirac-institute / sorcha

An open-source community LSST Solar System Simulator
Other
15 stars 16 forks source link

Discovery submission date in miniDifi seems to be a day off #959

Closed astronomerritt closed 1 week ago

astronomerritt commented 1 month ago

Essentially, when presented with the following input:

min_observations = 2
min_angular_separation = 0.5
max_time_separation = 0.0625
min_tracklets = 3
min_tracklet_window = 15
detection_efficiency = 1

# create object that should definitely be linked
obj_id = ["pretend_object"] * 6
field_id = np.arange(1, 7)
times = [60000.03, 60000.06, 60005.03, 60005.06, 60008.13, 60008.14]
ra = [142, 142.1, 143, 143.1, 144, 144.1]
dec = [8, 8.1, 9, 9.1, 10, 10.1]

observations = pd.DataFrame(
    {"ObjID": obj_id, "FieldID": field_id, "fieldMJD_TAI": times, "RA_deg": ra, "Dec_deg": dec}
)

linked_observations = PPLinkingFilter(
    observations,
    detection_efficiency,
    min_observations,
    min_tracklets,
    min_tracklet_window,
    min_angular_separation,
    max_time_separation,
)

This returns:

 ObjID  FieldID     fieldMJD_TAI    RA_deg  Dec_deg     object_linked   date_linked_MJD
pretend_object  1   60000.03    142.0   8.0     True    60007.0
pretend_object  2   60000.06    142.1   8.1     True    60007.0
pretend_object  3   60005.03    143.0   9.0     True    60007.0
pretend_object  4   60005.06    143.1   9.1     True    60007.0
pretend_object  5   60008.13    144.0   10.0    True    60007.0
pretend_object  6   60008.14    144.1   10.1    True    60007.0

However, we would actually expect date_linked_MJD to be 60008.0 here.

The last two observations take place past midnight in Chile, so it's not a day/night in Chile issue. I've tried a few numbers and the linking date seems to be consistently one day before when you'd expect.

mjuric commented 1 month ago

This may be about the definition of the "date_linked_MJD" column. I've been defining it as the night observing that enabled the linkage (*), not the calendar date/time of when the linkage itself was done.

For Rubin, we define a "night" in Chile as starting at 17hrs UTC, and ending the next day at 16:59:59.9999... The integer MJD we use is the one for the beginning of the night. So in the example above, 60008.14 would belong to night 60007, so I think having 60007 as the date of linking is OK.

(*) there are some subtleties here about when an old tracklet drop out of the linking window on a night when no new observations are acquired, and that /remova/ enables the linking. Ask me on at a meeting for details.

mschwamb commented 1 month ago

I think that's it. I didn't go until 17 hours the next time. I was suggesting tests that would change the time after sunrise in Chile

See https://github.com/dirac-institute/sorcha/pull/961

astronomerritt commented 1 month ago

Setting the last tracklet to 60008.81, 60008.82 (which is UTC ~19:25/19:40) does indeed change the discovery date to 60008.

However, setting the last tracklet to occur between roughly 60008.60 and 60008.81 (so 14:25 UTC and 19:25 UTC) causes the code to fail with an assertion error. I assume this is some sort of night boundary checking in miniDifi. Since this would be the middle of the day in Chile I doubt this is a problem.

I expect this issue can be closed.

astronomerritt commented 1 month ago

And thanks Mario for the explanation!

mschwamb commented 1 month ago

Also we need this highly documented in the code so it makes sense to future us when Mario is busy and we cannot figure out why this is doing what it is doing even though we know it is right.

astronomerritt commented 1 month ago

I can expand the comment here to explain the discovery submission date properly, would that do?

https://github.com/dirac-institute/sorcha/blob/45b5a4e102f5b7f4a2a6a771b19baf37695be69e/src/sorcha/modules/PPLinkingFilter.py#L93-L99

mschwamb commented 1 month ago

Let's have @mjuric deal with it and decide where it goes and add it to the doc strings as well

mjuric commented 4 weeks ago

However, setting the last tracklet to occur between roughly 60008.60 and 60008.81 (so 14:25 UTC and 19:25 UTC) causes the code to fail with an assertion error. I assume this is some sort of night boundary checking in miniDifi. Since this would be the middle of the day in Chile I doubt this is a problem.

@astronomerritt -- can you give me more details (where did you make this change & what was the assertion)? This looks like something that needs fixing.

Edit: Ignore the question above -- I think i found it (60008.81 should be 60008.61 to trigger the assertion).

mjuric commented 4 weeks ago

The assertion that's triggered by a tracklet around ~60008.6 is this one:

>           assert np.all(
                (0.1 < phased) & (phased < 0.9)
            )  # quick check that we didn't screw up the night boundary
E           AssertionError

(this is from src/sorcha/modules/PPMiniDifi.py).

I added this code as a guardrail while developing, as there should be no observations around local noon for a typical telescope. MJD 60008.6 is around ~10am local time, which triggered the assertion.

I'm not 100% sure what to do about it... It's not very elegant (it should die with a nicer error message), but I'm inclined to leave it in for now as a) it catches mistakes where the user forgets to set the config variable for their telescope's local time, and b) we may need to change this anyway for satellites as the minidifi requirements have a concept of a "night" boundary for tracklets, which the satellites don't (we'll deal with that generalization later).

Bottom line: I'd leave the assertion in for now.

mschwamb commented 4 weeks ago

Note - we don't have telescope local time in the cofig file. Just the obs code. So Chile is hardwired in here somewhere.

mjuric commented 3 weeks ago

Ah, OK.

This should be easy to add. Two options:

N.b., current mini-difi defaults can be found here

I'll split this off as a separate issue.

mjuric commented 1 week ago

Resolved in #972