Fixes for MSL #580
Fixes for MSL #580
Conversation
Post-conflict fixes More conflict merging More reconciliation with recent ale More reconciliation Fix for pixel size
This shows orthoimages from SOL0597 and SOL0603, including full-resolution F nav images and lower resolution D nav images (PDS has both kinds, and these are acquired at different times). SOL0603 are the darker image group in the lower portion. The data is kind of messy, but what is important is that there is agreement across days. For comparison, here's the rover path: 
SOL0597 correspond to rover being at the top dot, and SOL0603 at the bottom dot. (https://mars.nasa.gov/msl/mission/where-is-the-rover/) |
Undo some tests
oleg-alexandrov
force-pushed
the
msl2
branch
from
94320a1 to
cb764d6
Compare
|
I fixed the MSL unit tests except the "nadir" one. That I did not touch, it says it cannot find some driver. |
|
Hi @oleg-alexandrov, I've been looking into the failing unit test, I was able to get a little more specific error message shown below:
Hopefully this helps, I'll still be working on figuring out the issue. Update: Set |
|
Christine, thank you. Yeah, I got to that too. It appears that the computation of the rover-to-sensor offset that I added does not play nice with the nadir mode logic. That mode does not assume this offset anyway, so I put a fix to bypass this calculation in that case. After updating the reference result it compares against as well, now the test passes. |
|
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 #580 +/- ##
==========================================
- Coverage 16.24% 16.17% -0.07%
==========================================
Files 56 56
Lines 6119 6144 +25
==========================================
Hits 994 994
- Misses 5125 5150 +25 ☔ View full report in Codecov by Sentry. |
|
Changelog note added. |
| @@ -380,7 +380,7 @@ def extract_exact_ck_times(observStart, observEnd, targetFrame): | |||
|
|
|||
| return times | |||
|
|
|||
| def compute_time_dependent_rotiations(self, frames, times, time_bias): | |||
| def compute_time_dependent_rotations(self, frames, times, time_bias): | |||
There was a problem hiding this comment.
This change is technically API breaking but I think we are planning to pull a 1.0 release, so it should be fine.
There was a problem hiding this comment.
This is an internal function, used only in that file.
There was a problem hiding this comment.
I agree that it is an internally used function but a user could access the function if they wanted to and compute other frame rotations on a framechain object
There was a problem hiding this comment.
This also came up in the usgscsm repository; with no consensus, as I see.
There should be a clear policy, as to what parts of a library are the user-facing API, which must be guaranteed to be kept fixed, or at least not modified without a good reason, and which part is internal implementation. Constraining all internal functions is not possible.
There was a problem hiding this comment.
At least in ISIS, it's any function that a user has access to. Since that is C/C++ code that is more enforceable. Typically in python a "private" function is defined with a leading _ so that is something we could do in our python libraries. Similarly we should make functions private in USGSCSM that should not be public facing API
This is a proposed fix for MSL cameras in ALE. The pose hack in the previous attempt was removed and this is based off the latest code.
The root problem seems to have been that the Cahvor camera needs to be relative to the rover body, and not to a mast frame (so relative to MSL_ROVER rather than MSL_RSM_HEAD Naif frame). This eliminates the strange poses seen before and is also suggested by the .LBL file, which says REFERENCE_COORD_SYSTEM_NAME = "ROVER_NAV_FRAME".
Another fix is not to use the "light time correction" when querying Spice for the rover position. This correction results in the vertical position of the rover steadily and artificially changing throughout the solar day. Note that the LRO driver (LroLrocNacPds3LabelNaifSpiceDriver) also uses no such correction and says it is not accurate in that context.
Lastly, the sensor position was updated to incorporate the offset from the rover body to Cahvor camera center. This results in the correct stereo baseline for NAV cameras.
This was tested with left and right NAV cameras (full and low resolution sets) and left and right MAST cameras.
The above figure shows the orthoimages produced after running stereo on all full resolution NAV camera pairs (26 pairs) for SOL 603. No bundle adjustment was done, these are poses queried from Spice only. The lightning changes, and there are holes due to occlusion due to boulders and rover body, but it can be seen that all images are consistent. Consistency is maintained after the rover changes position (top portion).
The above figure shows the colorized DEMs for the same dataset as above. Blue = -5039.506 meters and red = -5033.298 meters. No blending of DEMs was done, these are individually created and overlaid. Their quality varies. It can be seen that the DEMs are all consistent, and the height is larger on the upper-left where there is a hill.
The above figure shows MAST cam image 0603ML0025450040301380C00_DRCL (colored) overlaid on top of NAV cam image NLB_451026746EDR_F0311094NCAM00271M1 (gray) when both are mapprojected onto the DEM obtained from this left nav cam image and corresponding right nav cam image. Both are for SOL 603. They agree. This shows that the MAST and NAV cameras are consistent.
Also tested was the MAST cam stereo pair 0706ML0029980010304577C00_DRCL 0706MR0029980000402464C00_DRCL for SOL 0706.
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: