Hermite Position Reduction - #731
Conversation
Kelvinrr
left a comment
Kelvinrr
left a comment
There was a problem hiding this comment.
Biggest concern is the unit question, just some clarity needed there.
| def sensor_position(self): | ||
| """ | ||
| CaSSIS sets LT_SURFACE_CORRECT with LIGHTTIME_CORRECTION=LT+S, so ISIS | ||
| applies the surface light-time correction. The shared | ||
| NaifSpice.sensor_position samples the target body at the raw ephemeris | ||
| time in that branch, but ISIS samples it at the surface-light-time | ||
| adjusted time (ephem - obs_tar_lt + radius_lt); the body moves along its | ||
| orbit during that interval, which otherwise leaves a constant | ||
| tens-of-meters camera-center bias versus ISIS. This override applies that | ||
| for CaSSIS only, so the shared path is unchanged for every other sensor. |
There was a problem hiding this comment.
This is related to adding the TGO_CASSIS specific check in the sensor_position property. Currently, we are maintaining two implementations of the light time correction sensor_position code, one in the base Driver class the other in the TGO_CASSIS concrete driver. Currently, there is only one driver but it would need the duplicate implementation if any new drivers are added for TGO_CASSIS that use the NaifSpice mixin.
| altitude = (middle_magnitude - radius) * 1000 | ||
| tol = self.pixel_size * altitude / self.focal_length / 100. |
There was a problem hiding this comment.
probably need comments on conversions here. Altitude is being converted from KM to meters, I assume, but I do not know what deviding the tol by 100 is doing. Altitude is in M but tol is in some other unit?
There was a problem hiding this comment.
Dividing by 100 is just setting the tolerance to 1/100th of the computed pixel resolution
| "spk_table_start_time": 302228504.36824864, | ||
| "spk_table_end_time": 302228558.33807606, | ||
| "spk_table_original_size": 25, | ||
| "spk_table_original_size": 2, |
There was a problem hiding this comment.
maybe I forgot what this value was, but if it's the number of ephems, it seems the actual number in the ISD did not change.
There was a problem hiding this comment.
This is a weird case where we are using a cropped LRO cube that has been reduced to one line but retains the InstrumentPosition table as if it was the full cube. So the change to the formatter is changing the spk_table_original_size in the resulting ISD.
If we wanted to get the actual value in there from the ISIS table:
Name = InstrumentPosition
StartByte = 117778
Bytes = 1400
Records = 25
ByteOrder = Lsb
CacheType = HermiteSpline
SpkTableStartTime = 302228504.36825
SpkTableEndTime = 302228558.33808
SpkTableOriginalSize = 816.0
Description = "Created by spiceinit"
Kernels = $lro/kernels/spk/fdf29r_2009182_2009213_v01.bsp
It should be 816.0 but I can't think of an easy way to transfer it
There was a problem hiding this comment.
I figured it was something like that.
| if self.instrument_id == "TGO_CASSIS": | ||
| adjusted_time = ephem - obs_tar_lts + radius_lt | ||
| ssb_tars_kwargs["startEt"] = adjusted_time[0] | ||
| ssb_tars_kwargs["stopEt"] = adjusted_time[-1] |
There was a problem hiding this comment.
Is this to avoid overwriting the entire function? Is there a clean way to use ALE's paradigm instead of an if statement?
There was a problem hiding this comment.
I don't think this would be a property that a user would pass in. The way I see handling it without an explicit check is to add a new property on the base NaifSpice mixin that is a boolean for adjusted_time and default it to false. Then the concrete drivers could override it as necessary. It would only be used by concrete Cassis drivers that use the NaifSpice mixin
2 participants
This PR adds the ability to reduce the number of ephemeris positions in ALE using the exsiting hermite spline creation code in the
Statesclass in the C++ side of ALELicensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: