search

tart-telescope / tart2ms

Convert TART data to measurement set format
GNU General Public License v3.0
1 stars 1 forks source link

Celestial catalogs, arbitrary celestial phasing, solar system phasing and minor bug fixes #48

Closed bennahugo closed 1 year ago

bennahugo commented 1 year ago

Closes #46 Closes #47 Closes #45

Implements phasing (fringe stopping) for:

bennahugo commented 1 year ago

@ifornax please see this branch. I realize now the data I pulled are only on my laptop, so I will upload for you soon. This branch is orders of magnitude faster than the previous one where the ::SOURCES subtable took ages

@tmolteno the only real bottleneck now is the TART API calls for satellite positions. I don't think JSON is the format to use -- at least not per field text files -- it takes about a minute to download them for every minute of observation in SA. I think this should rather be distributed as a binary blob

bennahugo commented 1 year ago

@tmolteno I left some comments to make this review easier. Still think I should put in filters to filter UTC spans for stitching purposes

bennahugo commented 1 year ago

No. The w coordinate in the solar system case is special in that it is not tracking an object by the hour angle clock alone - the object changes in Ra Dec on a constant basis so we need to change this per dump - same as with zenith. For Ra Dec positions the uvw system is only initialized once for a constant position on the celestial sphere

On Fri, 23 Dec 2022, 22:29 Tim Molteno, @.***> wrote:

@.**** commented on this pull request.

In tart2ms/tart2ms.py https://github.com/tart-telescope/tart2ms/pull/48#discussion_r1056636902 :

  • fn = phase_center_policy.replace("rephase-","")

  • 3CRR currently only catalog with special names

  • catalog_positions = catalog_reader.catalog_factory.from_3CRR(fluxlim15=0.0)
  • named_positions = read_known_phasings()
  • fsrc = list(filter(lambda x: x.name == fn, catalog_positions))
  • if len(fsrc) > 0:
  • centroid_direction = np.array([[fsrc[0].rarad, fsrc[0].decrad]]).reshape(1, 2)
  • else: # otherwise it is in the named positions list
  • fsrc = list(filter(lambda x: x['name'].upper() == fn, named_positions))
  • if len(fsrc) == 0:
  • raise RuntimeError(f"Unknown named source {fn}")
  • if fsrc[0]['position']['FRAME'] == "Special Body":
  • centroid_direction = np.empty((len(obstime), 2), dtype=float)
  • for ti, tt in enumerate(obstime):
  • body = ac.get_body(fn, tt, location=location)
  • centroid_direction[ti, :] = np.deg2rad(np.array([body.icrs.ra.value, body.icrs.dec.value]))

I don't understand here. Surely this is true regardless of what is being tracked. In each case the object is moving in the telescope frame, and uvw coordinates will need to be recalculated for each integration? This will happen with celestial objects, solar system objects and satellites?

— Reply to this email directly, view it on GitHub https://github.com/tart-telescope/tart2ms/pull/48#discussion_r1056636902, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6VXB22QBGX5UNOGIXTWOYDRRANCNFSM6AAAAAATEUDUXI . You are receiving this because you authored the thread.Message ID: @.***>

bennahugo commented 1 year ago

Not a sure I agree here. The telescope time is either UTC (or better UT1) or LST. If we rely on locals it creates an additional level where errors can creep in, e.g. misconfugurations in system date time

On Fri, 23 Dec 2022, 22:20 Tim Molteno, @.***> wrote:

@.**** commented on this pull request.

We should avoid this date parsing code. parsedateISO8601 will fail if the user specifies a timezone (which should never be punished). We should use

from tart.util.utc import to_utc dt = dateutil.parser.parse(x) utcdt = to_utc(dt)

Then we don't over roll our own date parser (one of the fundamental laws of programming)!

— Reply to this email directly, view it on GitHub https://github.com/tart-telescope/tart2ms/pull/48#pullrequestreview-1229387696, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6UEPOEWJFB2RM3GC4LWOYCQBANCNFSM6AAAAAATEUDUXI . You are receiving this because you authored the thread.Message ID: @.***>

bennahugo commented 1 year ago

I would just discard tzinfo if parsed with dateutils for comparison sake, but I agree we can switch to the dateutils parser - I didn't want to add an additional package when I wrote this.

On Fri, 23 Dec 2022, 22:46 Benna Hugo, @.***> wrote:

Not a sure I agree here. The telescope time is either UTC (or better UT1) or LST. If we rely on locals it creates an additional level where errors can creep in, e.g. misconfugurations in system date time

On Fri, 23 Dec 2022, 22:20 Tim Molteno, @.***> wrote:

@.**** commented on this pull request.

We should avoid this date parsing code. parsedateISO8601 will fail if the user specifies a timezone (which should never be punished). We should use

from tart.util.utc import to_utc dt = dateutil.parser.parse(x) utcdt = to_utc(dt)

Then we don't over roll our own date parser (one of the fundamental laws of programming)!

— Reply to this email directly, view it on GitHub https://github.com/tart-telescope/tart2ms/pull/48#pullrequestreview-1229387696, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6UEPOEWJFB2RM3GC4LWOYCQBANCNFSM6AAAAAATEUDUXI . You are receiving this because you authored the thread.Message ID: @.***>

tmolteno commented 1 year ago

Not a sure I agree here. The telescope time is either UTC (or better UT1) or LST. If we rely on locals it creates an additional level where errors can creep in, e.g. misconfugurations in system date time … On Fri, 23 Dec 2022, 22:20 Tim Molteno, @.> wrote: @*.*** commented on this pull request. We should avoid this date parsing code. parsedateISO8601 will fail if the user specifies a timezone (which should never be punished). We should use from tart.util.utc import to_utc dt = dateutil.parser.parse(x) utcdt = to_utc(dt) Then we don't over roll our own date parser (one of the fundamental laws of programming)! — Reply to this email directly, view it on GitHub <#48 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4RE6UEPOEWJFB2RM3GC4LWOYCQBANCNFSM6AAAAAATEUDUXI . You are receiving this because you authored the thread.Message ID: @.>

No, if the user does provide a timezone, we should not just ignore it. We can either throw an error if it the timezone isn't UTC or convert to UTC. The telescope time is just a time, it is a bug if it doesn't include the timezone as UTC in there when it is reported by an API, because errors creep in when we start assuming timezones without requiring them.

tmolteno commented 1 year ago

I would just discard tzinfo if parsed with dateutils for comparison sake, but I agree we can switch to the dateutils parser - I didn't want to add an additional package when I wrote this. … On Fri, 23 Dec 2022, 22:46 Benna Hugo, @.> wrote: Not a sure I agree here. The telescope time is either UTC (or better UT1) or LST. If we rely on locals it creates an additional level where errors can creep in, e.g. misconfugurations in system date time On Fri, 23 Dec 2022, 22:20 Tim Molteno, @.> wrote: > @.** commented on this pull request. > > We should avoid this date parsing code. parsedateISO8601 will fail if the > user specifies a timezone (which should never be punished). We should use > > from tart.util.utc import to_utc > dt = dateutil.parser.parse(x) > utcdt = to_utc(dt) > > Then we don't over roll our own date parser (one of the fundamental laws > of programming)! > > — > Reply to this email directly, view it on GitHub > <#48 (review)>, > or unsubscribe > https://github.com/notifications/unsubscribe-auth/AB4RE6UEPOEWJFB2RM3GC4LWOYCQBANCNFSM6AAAAAATEUDUXI > . > You are receiving this because you authored the thread.Message ID: > **@.***> >

tart2ms uses tart package which depends on dateutil already