V2.0 obs source #108
V2.0 obs source #108
Conversation
| def _within_radius( | ||
| detections: detections.PointSourceDetections, | ||
| ra: float, | ||
| dec: float, | ||
| radius: float, | ||
| ) -> detections.PointSourceDetections: | ||
| """ | ||
| Return the detections within a given radius of a given ra and dec | ||
| """ | ||
| sdlon = np.sin(detections.ra.to_numpy() - ra) | ||
| cdlon = np.cos(detections.ra.to_numpy() - ra) | ||
| slat1 = np.sin(dec) | ||
| slat2 = np.sin(detections.dec.to_numpy()) | ||
| clat1 = np.cos(dec) | ||
| clat2 = np.cos(detections.dec.to_numpy()) | ||
|
|
||
| num1 = clat2 * sdlon | ||
| num2 = clat1 * slat2 - slat1 * clat2 * cdlon | ||
| denominator = slat1 * slat2 + clat1 * clat2 * cdlon | ||
|
|
||
| distances = np.arctan2(np.hypot(num1, num2), denominator) | ||
|
|
||
| mask = distances <= radius | ||
| return detections.apply_mask(mask) |
There was a problem hiding this comment.
Obviously ought to be in adam_core but this works for now
| left_keys=detections.exposure_id, | ||
| right_keys=exposures.id, | ||
| ) | ||
|
|
There was a problem hiding this comment.
We have already discussed this offline, but for the record, a lightweight class that wraps a quivr linkage is an excellent idea.
|
|
||
| """ | ||
|
|
||
| def __init__(self, radius: float, all_observations: Observations): |
There was a problem hiding this comment.
You've made me a type hinting stickler now. Do __init__s normally have a return type?
There was a problem hiding this comment.
Nice catch! __init__ always returns None, and is assumed to do so. There is a lengthy and boring thread on this at python/mypy#604, resolved in python/mypy#5677.
There's nothing wrong with explicitly adding -> None there though.
You might be a bit surprised that it returns None and not, say, Self. That's because __new__ is the actual constructor function which returns the new class instance. __init__ is actually a hook that gets called to initialize the new instance, and its return value is discarded.
| y=[0], | ||
| z=[0], | ||
| vx=[0], | ||
| vy=[6.14], |
There was a problem hiding this comment.
6.14 au per day is ~3% the speed of light. This test orbit is flying!
You could give it a y-velocity of np.sqrt(c.MU/r) if you wanted to make it a circular orbit instead of an unbound one.
There was a problem hiding this comment.
Hahahaha, yeah, that's what what I actually wanted! Thanks.
There was a problem hiding this comment.
(I'm doing 2𝜋/365.24 instead, because that's how my brain works, and they're the same up to 5 digits of precision :))
|
|
||
| # Approximately pi/4 (~0.78) of the detections should be in the 0.5 degree | ||
| # radius | ||
| assert len(have.detections) > 0.75 * len(fixed_observations.detections) |
There was a problem hiding this comment.
And also false! Spherical geometry is super weird and I empirically get more like 95% within the circle. I'm not sure what's going on.
|
I think this is good to merge, but it would land on your test orbit branch @moeyensj . Let's merge that first into v2.0 rather than stacking too deeply. |
| @@ -78,8 +78,7 @@ def __init__( | |||
| else: | |||
| self.orbit_id = uuid.uuid4().hex | |||
|
|
|||
| if object_id is not None: | |||
| self.object_id = object_id | |||
| self.object_id = object_id | |||
There was a problem hiding this comment.
I won't touch this in #107 since you address it here.
(merge base is #107, take care!)
Add ObservationSource which loads observations and filters them to ones that match a test orbit. This is an interface that describes how data can make its way into THOR. We can implement more ObservationSources in the future, but these two are fundamental.