Assist ephem #7
Assist ephem #7
Conversation
There was a problem hiding this comment.
I'd definitely prefer this on the EphemerisMixin in adam-core. I saw that there was some kind of error in slack. I'm guessing some kind of inheritance issue?
I'd like to see linting pass and we should have at least one unit test for this, perhaps where we can hardcode some values from a sorcha run or from a JPL ephemeris tool where we know it fails without the light time correction.
You should be able to run pdm run check to run the linting and unit tests.
| origin_out=OriginCodes.SOLAR_SYSTEM_BARYCENTER, | ||
| ), | ||
| ) | ||
| observers_barycentric = observers.set_column( |
There was a problem hiding this comment.
This can be moved to outside the for-loop, I think. Probably negligible in most cases, but adds some compute time if there are lots of orbits or lots of observers.
| dlt = 1 | ||
| while dlt > lt_tol: | ||
| observer_position = observer.coordinates.values[0,:3] | ||
| orbit_i = orbit |
There was a problem hiding this comment.
What's the purpose of renaming here, since these will be the same object in memory? I was confused because we concat the orbit instance at the end but we are modifying orbit_i and I had to realize they were the same.
| for i, (orbit, observer) in enumerate(zip(orbits, observers)): | ||
| lt0 = 0 | ||
| dlt = 1 | ||
| while dlt > lt_tol: |
There was a problem hiding this comment.
I don't see a any handler for max-attempts. Is this loop guaranteed to converge? (I honestly don't know).
| ephemeris_total = VariantEphemeris.empty() | ||
|
|
||
| for orbit in orbits: | ||
| propagated_orbits = self._propagate_orbits(orbit, observers.coordinates.time) |
There was a problem hiding this comment.
| propagated_orbits = self._propagate_orbits(orbit, observers.coordinates.time) | |
| propagated_orbits = self.propagate_orbits(orbit, observers.coordinates.time) |
We should be calling the public API here, I think.
|
|
||
| t1 = t0 - lt | ||
| t1 = Timestamp.from_mjd([t1], scale="tdb") | ||
| orbit_propagated = self._propagate_orbits(orbit, t1) |
There was a problem hiding this comment.
| orbit_propagated = self._propagate_orbits(orbit, t1) | |
| orbit_propagated = self.propagate_orbits(orbit, t1) |
| object_id=propagated_orbits_barycentric.object_id, | ||
| coordinates=spherical_coordinates, | ||
| weights=weights, | ||
| weights_cov=weights_cov, |
There was a problem hiding this comment.
Does VariantEphemeris need the light_time column?
| orbit_id=propagated_orbits_barycentric.orbit_id, | ||
| object_id=propagated_orbits_barycentric.object_id, | ||
| coordinates=spherical_coordinates, | ||
| light_time=light_time, |
There was a problem hiding this comment.
I notice we're not using the aberrated_coordinates column. I'm guessing it was decided that was too confusing?
No description provided.