-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lro Wac fix #595
Lro Wac fix #595
Conversation
Further edits to the lro wac ephemeris times Update lro wac load test Refined sliced kernels to be much smaller
@@ -115,7 +115,7 @@ def ephemeris_time(self): | |||
ephemeris times split based on image lines | |||
""" | |||
|
|||
return np.arange(self.ephemeris_start_time + (.5 * self.exposure_duration), self.ephemeris_stop_time + self.interframe_delay, self.interframe_delay) | |||
return np.arange(self.ephemeris_start_time, self.ephemeris_stop_time, self.interframe_delay) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine this was just changed to match ISIS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhat, rather than apply the + (.5 * self.exposure_duration)
in the pushframe sensor type, the start time should be updated on the driver. If you look at the ephemeris_start_time
on the LroLrocWacIsisLabelNaifSpice
driver is has the + (.5 * self.exposure_duration)
@@ -162,7 +162,7 @@ def ephemeris_stop_time(self): | |||
: double | |||
Center ephemeris time for an image | |||
""" | |||
return self.ephemeris_start_time + (self.interframe_delay) * (self.num_frames - 1) + self.exposure_duration | |||
return self.ephemeris_start_time + (self.interframe_delay * self.num_frames) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the math here was weird relative to ISIS
@@ -1192,7 +1191,7 @@ def num_frames(self): | |||
: int | |||
Number of frames in the image | |||
""" | |||
return self.image_lines // (self.framelet_height // self.sampling_factor) | |||
return (self.image_lines // (self.framelet_height // self.sampling_factor)) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did it need a +1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was also an ISIS motivated change, the LroWideAngleCamera in ISIS calculated one extra frame. I am not sure why as the PushFrameCameraDetectorMap logic is hard to follow
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #595 +/- ##
==========================================
- Coverage 15.49% 15.48% -0.01%
==========================================
Files 57 57
Lines 6417 6419 +2
==========================================
Hits 994 994
- Misses 5423 5425 +2 ☔ View full report in Codecov by Sentry. |
Updates the LRO Wac driver and push frame timing.
Licensing
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: