NeoGeographyToolkit / StereoPipeline

The NASA Ames Stereo Pipeline is a suite of automated geodesy & stereogrammetry tools designed for processing planetary imagery captured from orbiting and landed robotic explorers on other planets.
Apache License 2.0
478 stars 168 forks source link

Reporting a Bug: WARNING: GCPs are over 100 km from the other points. Are your lat/lon GCP coordinates swapped? #404

Closed sxsong1207 closed 1 year ago

sxsong1207 commented 1 year ago

Hello,

I hope this message finds you well. I wanted to bring to your attention a small bug that I discovered in the latest version of the software. This bug consistently generates a warning message that, although harmless, appears to be unnecessary.

Bug Description: Whenever a GCP (Ground Control Point) file is provided, the software generates a warning message without any other discernible effects. The warning message does not provide any useful information and seems to be an unintended behavior.

Bug Trigger: The bug is triggered whenever a GCP file is given as input during the processing.

Root Cause: Upon analyzing the code in the file BundleAdjustCamera.cc, specifically at lines 221 and 249, I noticed that the variable gcp_count is initially computed correctly in line 221. However, in line 249, the value of gcp_count is changed, resulting in an incorrect value assignment.

Proposed Solution: To resolve this issue, I suggest removing line 249 (++gcp_count;) as it appears to be the incorrect modification causing the bug.

Please let me know if you require any further information or assistance in resolving this bug. I would be happy to provide any additional details or help with testing a fix.

Thank you for your attention to this matter, and I look forward to seeing this bug addressed in a future release.

oleg-alexandrov commented 1 year ago

Dear Shaun, that was a great catch, and I very much appreciate you taking the time to look at code and understand the issue. I put a fix, as per your suggestion, and now the means are comparable and it made the problem go away. I also removed a lot of duplicate code in there as the 2nd problematic block logic can be done in the first block to start with.

The build 2023-06-14 at https://github.com/NeoGeographyToolkit/StereoPipeline/releases will have the fix in a day.