Add ShadowCam ISIS#6011
Conversation
- Implement shadowcamcal - Implement shadowcam2isis - Implement ShadowCamCamera
- Move all functions into a ShadowCam sub-namespace - Move function docstrings into the header and correct them - Add missing includes, remove unused includes, and add include guard - Remove using namespace std declaration and scope any standard library members - Modify GetTdiFactor to throw an error if an invalid TDIDirection value is encountered, rather than ignoring it and defaulting to 0 - Modify ToLower to leverage the functionality already available in QString
- Remove unnecessary log parameter from shadowcamcal - Add shadowcamcal overload accepting an input cube and ui - Correct typos and make variable names more consistent - Simplify code where possible - Remove unnecessary using namespace std - Add missing includes and remove unused ones
- Move shadowcamcal's WriteCube helper function into a ShadowCam sub-namespace within shadowcamcal.cpp and shadowcamcal.h - Improve variable naming consistency and simplify where possible - Update references to WriteCube accordingly
- Rename DarkSubtraction to SubtractDark and move it to a sub-namespace within shadowcamcal - Update references to DarkSubtraction - Add docstring to dark subtraction function - Simplify dark subtraction function where possible and make style and formatting consistent - Add missing scope prefixes and headers - Remove unnecessary headers - Remove old dark subtraction function
- Rename FlatFieldCorrection to CorrectFlatfield, move it to a sub-namespace in shadowcamcal, and refactor it - Improve code styling, readability, and complexity where possible - Update references to old function where applicable
- Rename GainCorrection to CorrectGain, move it to a sub-namespace in shadowcamcal, and refactor it - Improve style, consistency, and complexity where possible
- Rename radiance correction from RadianceCoefficients to CorrectRadiance and move it into a sub-namespace in shadowcamcal - Refactor radiance correction to improve readability, consistency, and complexity where possible
|
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_6011". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
|
Thanks for getting this in! Some followup questions: any additional supporting files that are needed for working shadowcam? Looking at calibration I dont see an obvious need for an additional calibration file. But where do we get SPICE kernels? Is it somewhere in |
|
A quick comment about the remaining time offset: I believe it is due to a limit in the precision of data reported in the PDS file headers for ShadowCam. The difference is in the 7th decimal place, and the various time values in the PDS header are "only" reported to 6 decimal places. In the prototype code, meanwhile, the various adjustments go out to ~15-20 decimal places. And yes, the KPLO kernels are available on the same rsync server as LROC. Just change the /lro/ to /kplo/ ( |
|
Any objections to pushing directly to this to add tests? @cordellmichaud |
I have no objections. Feel free to push to it directly. |
|
@cordellmichaud or @rvwagner small question regarding calibrated vs uncalibrated data and ingestion. The I just want to clarify as it seems like the only things this PR needs is a few error checks and documentation tweeks |
|
Yeah, I suspect that is left over from the NAC documentation that was probably adapted (possibly before we decided on the final PDS format?). Probably should simply mention that CDR and RDRs can be used directly, because they can. shadowcam2isis itself is kind of a make-work program, honestly. It exists because ISIS traditionally uses an xxx2isis program, and so we decided to stick the decompanding step of calibration in there to give it something to do. Our standalone tool goes straight from EDR to CDR in one program (which, to be clear, shadowcamcal cannot do, it needs shadowcam2isis first). |
|
Hi all, I put in a PR into @cordellmichaud's branch. It's enough of a change to the core apps that I wanted to give folks a chance to look over it |
Calibrated Error Check
|
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_6011". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
|
Pushed directly |
|
The build and test suite have started for your pull request. To view your build log, please reference the build with source version: "PR_6011". Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch. |
| /* | ||
| if (ShadowCam::IsSpecialPixelSHC(in[bufferIndex])) { | ||
| if (keepSpecial) | ||
| out[bufferIndex] = ShadowCam::Set8bitMaxMintoSpecialPixelsHIS4LIS4(in[bufferIndex]); | ||
| else { | ||
| // Special pixel handling | ||
| cout << "WARNING: Special pixels set to 0 or 255." << endl; | ||
| out[bufferIndex] = static_cast<float>(ShadowCam::Set_LIS_HIS_SpecialPixelsTo_0_255(in[bufferIndex])); | ||
| } | ||
| } | ||
| else { | ||
| // Decompanding non-special pixels | ||
| out[bufferIndex] = static_cast<float>(decompanding_table[(uint16_t) in[bufferIndex]]); | ||
| #if (out[bufferIndex] < 0){ | ||
| # cout << "Value is less than zero for line: " << std::to_string(in.Line()) << ", pixel: " << std::to_string(bufferIndex) << "at(i): " << std::to_string(in.at(bufferIndex)) << endl; | ||
| }*/ |
4 participants
Description
This PR adds the ShadowCamCamera and ShadowCamDistortionMap implementations, as well as the shadowcam2isis and shadowcamcal apps. They are in a working state in ISIS 8.3.0, but may need updates to become compatible with the latest ISIS. The apps also need ISIS tests.
Additionally, shadowcam2isis + shadowcamcal together are slower than our standalone calibration tool, and have room for significant performance improvement. I believe the major reason for this slowdown is the way that shadowcamcal currently writes each calibration step result to a separate output cube in a temporary location, allowing for the WriteOutSteps option to enable writing those temporary step output cubes to non-temporary output cubes. This WriteOutSteps option is an unnecessary lingering debug feature, and by coupling the removal of the WriteOutSteps option with some refactoring of the calibration steps, I believe we can apply calibration steps more directly, skip temporary cube I/O, and significantly improve performance.
Related Issue
How Has This Been Validated?
campt line=1 sample=1).Types of changes
Checklist:
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: