DOI-USGS / ale

Abstraction Layer for Ephemerides (ALE)
Other
13 stars 33 forks source link

Bufixes for MSL sensor intrinsics #589

Closed oleg-alexandrov closed 10 months ago

oleg-alexandrov commented 10 months ago

This is a fix for a bug I made with MSL cameras.

This bug explains part of the problem of why ISIS is having a hard time with MSL data.

A corresponding bug was fixed in ASP. The build 2024-01-12 has that. An earlier ASP build will now give wrong result.

The root cause of the problem was that for MSL the zero datum is not beneath the rover, but above it, so some heuristics was failing.

Things were working with mapproject because two bugs were canceling each other out (since I developed both). No such luck with ISIS.

This was very carefully tested with many SOL00603 MAST and NAV images. The most relevant pair is:

SOL00603/NLB_451026746EDR_F0311094NCAM00271M1 MAST_0603/0603ML0025450040301380C00_DRCL

These can be used with the MSL DEM I shared with @acpaquette.

I am not sure a new changelog entry is needed, since the existing MSL work is already listed.

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:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

acpaquette commented 10 months ago

Sweet, thank you Oleg. Two things:

Tests will likely need to be fixed, just started the pipelines

Can we get another changelog entry under Fixes. Something along the lines of inverted pixel_size and pixel_size related properties in CAHVOR/MSL

oleg-alexandrov commented 10 months ago

Will do, in a bit.

On Fri, Jan 12, 2024 at 1:05 PM acpaquette @.***> wrote:

Sweet, thank you Oleg. Two things:

Tests will likely need to be fixed, just started the pipelines

Can we get another changelog entry under Fixes. Something along the lines of inverted pixel_size and pixel_size related properties in CAHVOR/MSL

— Reply to this email directly, view it on GitHub https://github.com/DOI-USGS/ale/pull/589#issuecomment-1889947373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDU3BNOW7HAXII5T2VPTLYOGQQFAVCNFSM6AAAAABBYUVWCSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBZHE2DOMZXGM . You are receiving this because you authored the thread.Message ID: @.***>

oleg-alexandrov commented 10 months ago

I put a fix to the tests. Only one test still fails:

tests/pytests/test_util.py::test_get_metakernels_no_alespiceroot

This is a strange one. It is supposed to warn if ALESPICEROOT is not set. I did not modify anything regarding that, so not sure.

Let us see if cloud tests pass.

Also added to changelog.

acpaquette commented 10 months ago

The 3.8 test is failing due to poor download speeds on linux. This looks good otherwise, I am going to merge this and deal with the failing ci in another PR

codecov-commenter commented 1 month ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 15.82%. Comparing base (ec88933) to head (db863c4). Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
ale/drivers/msl_drivers.py 0.00% 4 Missing :warning:
ale/base/type_sensor.py 0.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #589 +/- ## ======================================= Coverage 15.82% 15.82% ======================================= Files 56 56 Lines 6283 6283 ======================================= Hits 994 994 Misses 5289 5289 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.