search

DOI-USGS / usgscsm

This repository stores USGS Community Sensor Model (CSM) camera models
Other
26 stars 33 forks source link

Update SAR to the newest format #389

Closed oleg-alexandrov closed 2 years ago

oleg-alexandrov commented 2 years ago

This is a proposed fix for https://github.com/USGS-Astrogeology/usgscsm/issues/303.

Here's what got done.

I pulled the changes from that draft pull request, fixed some conflicts, and updated with latest changes.

I made the ellipsoid radii be meters instead of km (that was some gotcha). Then I made the SAR state generation logic be as similar to Linescan as possible, to the point of even changing notation and reordering blocks so that it is easier to ensure there's no mistake.

When it comes to the reference testcase, it was easier to just get a new testcase than to fix orbitalSar.json. Mucking with json is a royal pain.

I overwrote it with lsz_03821_1cd_xku_16n196_v1.json, which was created with the 'ale' format. This is a realistic testcase, part of a stereo pair, and is based on Randy's papers. I think it is a better testcase than before.

I tweaked the tests a bit to make them work, as can be seen in the code diffs.

The important part was to ensure that the results stayed very similar to before given all these changes. I validated this with usgscsm_cam_test and with ASP's own ISIS to CSM comparison test, which does what knoten does.

With usgscsm_cam_test, here are the stats before and after, when it got ran with --sample-rate 100.

Before

Norm of pixel errors after projecting from camera to ground and back. Min: 5.51818e-10 Median: 0.00032303 Max: 0.00136482 Count: 15504

After

Norm of pixel errors after projecting from camera to ground and back. Min: 3.33796e-08 Median: 0.000326576 Max: 0.000875109 Count: 15504

With ASP's own tool, which compares CSM to ISIS camera:

Before

cam1 to cam2 camera direction diff norm Min: 1.31646e-09 Median: 2.94076e-05 Max: 0.00011082

cam1 to cam2 camera center diff (meters) Min: 2.43515e-05 Median: 0.0610273 Max: 0.12405

cam1 to cam2 pixel diff Min: 2.55237e-05 Median: 0.493439 Max: 2.17519

cam2 to cam1 pixel diff Min: 0 Median: 0 Max: 0

After

cam1 to cam2 camera direction diff norm Min: 1.31623e-09 Median: 2.94064e-05 Max: 0.00011082

cam1 to cam2 camera center diff (meters) Min: 2.43514e-05 Median: 0.0610257 Max: 0.124033

cam1 to cam2 pixel diff Min: 2.55236e-05 Median: 0.493431 Max: 2.17519

cam2 to cam1 pixel diff Min: 0 Median: 0 Max: 0

Note that the numbers barely changed. Now, we'd expect better agreement of ISIS to CSM here. That's an issue I will look into. I just checked now that in a testase I used a lot for a while, named LSZ_01636_1CD_XKU_09N120_S1.8bit, the agreement is better than here.

But that is a separate problem. What matters here is that not so much that the ISIS vs CSM agreement is perfect but that it did not change noticeably from before given that the json file format and model state generation changed. So, things are good.

oleg-alexandrov commented 2 years ago

I finally figured this out. Using gmtime is not enough. mktime() is a problem too. There's a summary further down.

oleg-alexandrov commented 2 years ago

Well, this was one nasty bug. The only reason it never came up so far is because all test cases used in the unit tests were fake, with an artificial January 2000 acquisition time. I added a real dataset, for SAR. As luck has it, its acquisition time was in the Summer.

This resulted in the following problem. The unit tests would pass on my machine, but fail on GitHub. The reason was that a Summer date, like 2020-08-16T08:52:18, can refer to a different time in a location which respects daylight saving time compared to one which does not. Took a long time to figure that out.

I modified the code to not use localtime() or mktime() which suffer from this problem. The calculation now starts with ephemeris time in seconds, adds to it the known time since epoch on January 1, 2000 12:00:00 AM GMT, and then calls gmtime().