-
Notifications
You must be signed in to change notification settings - Fork 173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NEAR/MSI msi2isis and MsiCamera model improvements #4887
Conversation
Rename the NEAR/MSI ingestion app main.cpp to msi2isis.cpp to covert to callable function
The old tests have been replaced by new FunctionalTests
msi2isis has been modified to accept expanded datasets * Accept images that have already been expanded to correct aspect (square pixels) * New TRIM option added to optional turn off trimming of borders * Modify SCLK start and stop times to be compatible with NAIF SCLK kernel format. This provides compatiblity for the sumspice application * Save original start/stop SCLK values to new keywords OriginalSpacecraftClockStartCount/ OriginalSpacecraftClockStopCount * Convert the app to a callable function
msi2isis funtional test and data * Converted all old app tests to new functional tests * Added label and data files * msi2isis has accompanying .lbl files as input but .gitignore excludes these files. Add explicit ignores for these *.lbl files
MsiCamera model modified so that sumspice works correcty * Use SpacecraftClockStartCount rather than StartTime to determine observation times for sumspice compatibility * Add IAKs to track in repository * Add light time and observer parameters to IAK version 2 * Bump camera version in plugin file due to new keywords loaded at spiceinit time
MsiCamera Gtest written to replace old version * Add new MsiCamera tests and Fixtures
NearMsiSerialNumber.trn update to add option keyword ControlSN to support use of simulated image in control processes
msiCamera unitTest.cpp in favor of NearMsiCameraCube.NearMsiCameraTest
Gonna get started on reviewing this, but I'll respond to a few things up front:
This is intentional, we don't want binary files accidentally added to the git repo as it makes for extremely large diffs and bloats the repo.
Which application tests are you talking about? The old msi2isis app tests should be removed as part of this PR.
Be sure to add a changelog entry under UNRELEASED / ADDED |
Actually this is the mission module tests in $ISISROOT/src/near.
I looked at the output and it appears these test require updating test data and I am uncertain how to make that change from my end. |
I can get those tests updated once this PR is merged. |
Just an FYI, after all the ctests have been ran on my system, there are some residual files that consistently remain (or marked as untracked). These files are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look good. I added a bunch of comments about how to improve the tests.
The MsiCamera requires a spiceinit cube, but .gitignore excludes .cub files. This commit addresses this issue by explicitly unignoring the Eros cube used for the test.
* Removed space between issue number and url in CHANGELOG.md msi2isis * Remove commented code * Simplify trimming. Let user decide uses for this option rather than assume it should not be used under certain conditions
I've moved the test cube to a label + ISD setup, it's not a huge savings here because the original cube is ~1MB and the label + ISD is ~15KB, but for larger images this is a big deal. I'm going to get the SPICE updated in our system now. |
Just pushed test modifications that address your comments/concerns. Please let me know if there is anything remaining preventing a merge. Thanks... |
NEAR/MSI support in ISIS has been improved and enhanced
Description
The NEAR/MSI Eros data has been improved via a NASA PDART proposal. This work includes improvements in remediation of hydrazine contamination and in support of new global color maps.
Modifications have been made to the
msi2isis
application that will ingest the new dataset that will be archived in the PDS. The new images have already been expanded to the standard size image 537x412 where lines are interpolated from the original dataset size of 537x244. Images of expanded size are accepted as long as all the expected values in the labels are valid. In addition, a new option called TRIM has been added that allows the user choose to trim the edges of the images. The remediated dataset has shown these data regions to be improved sufficiently to retain for mapping purposes. And the SCLK times are modified to be valid with used with the NEAR/MSI SCLK kernel. This modification, along with corresponding changes in the MsiCamera model, provides support for thesumspice
application to apply Gaskell SUMFILE ephemeris data updates.The
MsiCamera
model has been modified to use the SpacecraftClockStartCount SCLK values rather than StartTime which is in UTC. This change is required forsumspice
compatibly. Also, additional NAIF keywords were added to the ISIS IAK that provides support for improved geometry. These values are:This results in a new version of the NEAR/MSI IAK. Both of these IAKs are now stored/track in the repository in the $ISISROOT/src/near/objs/MsiCamera/iak directory. The new version,
msiAddendum002.ti
, needs to be copied to $ISISDATA/near/kernels/iak directory with this PR and prior to testing/release.Note the modifications to the
MsiCamera
model require a new version number bump since new keywords are being read from image labels (SpacecraftClockStartCount
) and IAK (as shown above).The serial number configuration file,
NearMsiSerialNumber.trn
, has also been updated to accomodate inclusion of an optional keyword we use for control-to-ground strategies.All
msi2isis
andMsiCamera
tests have been converted to the ISIS Google Test framework. As such, themsi2isis
application has been converted to a callable function. All old app and unit tests have been removed. This means that all test data files must be removed as well. Of note is I had to explicitly add (unignore) the NEAR/MIS *.lbl files to .gitignore as lbl files are ignored repo-wide.NOTE that I did not update the application test as I am unsure how to get these changes into the system. I think it only requires an update to the truth files once the tests have been rerun. Please advise/assist.
Related Issue
This PR address #4886.
Motivation and Context
We have produced a new set of NEAR/MSI Eros images where the remediation of hydrazine contamination has been improved. To support this dataset, modifications and updates to the NEAR/MSI ingestion application,
msi2isis
, and the camera model were needed. These modifications are included in this PR.How Has This Been Tested?
The following shows results of the ctest suite for these changes:
Screenshots (if appropriate):
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: