search

sot / proseco

Probabilistic star evaluation and catalog optimization
https://sot.github.io/proseco
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

Fiducial light ids don't match products #329

Closed jeanconn closed 3 years ago

jeanconn commented 4 years ago

It looks like proseco uses the indexes of the fids in fidpos as the fid "ids"

https://github.com/sot/proseco/blob/3621584b4713ef0aa047009ac2453df19a6028d0/proseco/characteristics_fid.py#L5

The FOT process (guide summary for example) uses the "unique" id for each fid light (only different for HRC-I, HRC-S)

https://github.com/sot/starcheck/blob/184f2209359be9c4bbcf6e3f0f8d8b2425f82209/starcheck/src/lib/Ska/Starcheck/Obsid.pm#L2090

For the process of, say, manually specifying a fid, should we set up proseco to match?

I think perhaps nothing is depending on this just now.

taldcroft commented 4 years ago

I don't see a substantial benefit to making this change, at least internally in how proseco handles fid IDs.

It would be easy enough to have some compatibility code on the front end so that if the user includes fid id=8 then that gets uniquely translated to HRC-I-2 (with a check that detector is HRC-I). The main use case I see is for the FOT manually specifying fids, but that will probably happen either zero or one times in the rest of the mission, and so we can just tell them what to do. But generally speaking less is more and having magic translations like this adds complexity / surprise.

If it helped anything we could add a property to the FidTable like ids_absolute that would give numbers in the range 1-14. But not obvious how that helps us out.

jeanconn commented 4 years ago

And I suppose the Matlab code is already doing "something" with these indexes.

On Fri, Jul 24, 2020, 5:27 AM Tom Aldcroft notifications@github.com wrote:

I don't see a substantial benefit to making this change, at least internally in how proseco handles fid IDs.

It would be easy enough to have some compatibility code on the front end so that if the user includes fid id=8 then that gets uniquely translated to HRC-I-2 (with a check that detector is HRC-I). The main use case I see is for the FOT manually specifying fids, but that will probably happen either zero or one times in the rest of the mission, and so we can just tell them what to do.

If it helped anything we could add a property to the FidTable like ids_absolute that would give numbers in the range 1-14. But not obvious how that helps us out.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sot/proseco/issues/329#issuecomment-663444724, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGBHVC4MHYVOMWB2Y2GALLR5FHVRANCNFSM4PFWBDXA .

taldcroft commented 3 years ago

I'm closing this. If/when there is a real problem that needs fixing we can reopen it.