Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Csmcsv #4446

Merged
merged 26 commits into from
May 12, 2021
Merged

Csmcsv #4446

merged 26 commits into from
May 12, 2021

Conversation

jessemapel
Copy link
Contributor

@jessemapel jessemapel commented May 11, 2021

Description

Updates BundleAdjust to output separate Images CSV files for each sensor. This solves the issue of mismatched columns when different images are solving for different parameters. Currently when different parameters sets are being solved for only the very first observation's parameters are represented in the header and the images CSV is malformed with variable numbers of columns on each line.

Related to #4410

Examples

This impacts CSM bundle, multi-sensor bundle with and without scconfig, and held images. Here's a few examples and how this impacts them:

CSM bundle

If you are bundle adjusting with multiple CSM sensors, then each CSM sensor will have a separate images CSV containing the updates for that sensor's image parameters.

Multi-Sensor bundle with scconfig

If you are bundle adjusting with multiple ISIS sensors using scconfig, then each separate entry in your scconfig will result in a separate images CSV. For example, the following scconfig:

Object = SensorParameters
  Group = MARS_ODYSSEY/THEMIS_IR
    CamSolve=accelerations
    OverExisting=yes
    Twist=yes
    camera_angles_sigma=.25
    camera_angular_velocity_sigma=.1
    camera_angular_acceleration_sigma=.01
    SPSolve=none
  EndGroup
  Group = VIKING_ORBITER_1/VISUAL_IMAGING_SUBSYSTEM_CAMERA_B
    camsolve=angles
    Twist=yes
    camera_angles_sigma=1.0
    SPSolve=none
  EndGroup
  Group = VIKING_ORBITER_2/VISUAL_IMAGING_SUBSYSTEM_CAMERA_A
    camsolve=angles
    Twist=yes
    camera_angles_sigma=1.0
    SPSolve=none
  EndGroup
EndObject
End

You will have 3 images CSV files:

  • bundleout_images_MARS_ODYSSEY_THEMIS_IR.csv
  • bundleout_images_VIKING_ORBITER_1_VISUAL_IMAGING_SUBSYSTEM_CAMERA_B.csv
  • bundleout_images_VIKING_ORBITER_2_VISUAL_IMAGING_SUBSYSTEM_CAMERA_A.csv

Multi-Sensor bundle without scconfig

For example if you are bundle adjusting a set of LRO NAC images that contains both NAC left and NAC right images you will still get separate images CSV files:

  • bundleout_images_LUNARRECONNAISSANCEORBITER_NACL.csv
  • bundleout_images_LUNARRECONNAISSANCEORBITER_NACR.csv

Even if you are solving for the same parameters for both sensors.

Held images bundle adjustment

If you are holding some images fixed using jigsaw's HELDLIST parameter then you will have one images CSV file your your held images called bundleout_images_held.csv and another images CSV file for the rest of your images. The name of your non-helf images CSV will again contain the spacecraft and sensor name.

Testing

The following tests are expected to fail due to updated truth data:

  • jigsaw_app_test_radar (Failed)
  • jigsaw_app_test_scconfig (Failed)
  • jigsaw_app_test_scconfigHeld (Failed)

> $(OUTPUT)/bundleout_scconfig_images_viking_2.csv;
$(CAT) bundleout_images_held.csv \
| perl -pe 's/(^|,|: )([^,:]+\/)([^,\/:]*\.)(net|cub)/\1\3\4/g' 2> /dev/null \
> $(OUTPUT)/bundleout_scconfig_images_held.csv;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good example of what the new output can look like. This test has THEMIS images, Viking 1 images, VIking 2 images, and a held image.

}

QFile m_file;

Copy link
Contributor Author

@jessemapel jessemapel May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a small fix to an unrelated issue where this file never got cleaned-up.

// Accumulate all of the images from previous observations
for (int obsIndex = 0; obsIndex < observationIndex; obsIndex++) {
imgIndex += m_statisticsResults->observations().at(obsIndex)->size();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make this part better, but it'll be a lot of work to modify the RMS vectors so I'm starting with this change.


for (int k = 0; k < numImages; k++) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This triple nested for loop isn't as bad as it looks because collectively it still just iterates over all the images. We just split the images up into 3 different tiers of sets. First we separate by instrument ID, then by observation, then by image.

@@ -13,6 +13,7 @@ find files of those names at the top level of this repository. **/
#include <QObject>
#include <QString>

#include "AbstractBundleObservation.h"
Copy link
Contributor Author

@jessemapel jessemapel May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be forward declared as a typedef but that's rather ugly, can be confusing, and even cause errors; so I just included it.

@@ -92,7 +92,6 @@ namespace Isis {
//! Map between cube serial number and BundleImage pointers.
QMap<QString, BundleImageQsp> m_cubeSerialNumberToBundleImageMap;
QStringList m_serialNumbers; //!< List of all cube serial numbers in observation.
QStringList m_parameterNamesList; //!< List of all cube parameters.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to have to store a whole bunch of strings for each observation, so I decided to just dynamically generate this for memory efficiency. It requires the same amount of code as generating and then saving them off anyway.

// so always do at least 1
int numberCamPosCoefSolved = std::max(
solveSettings()->numberCameraPositionCoefficientsSolved(),
1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like this but it's needed to preserve functionality. Ideally, this wouldn't be here and the twist would not be there if we weren't solving for it, but that's not the current functionality for the output files.

// update instrument ID to observation ptr map
// separate held observations out into their own entry
if (bundleObservation->numberParameters() == 0) {
m_instIdToObservationMap.insertMulti("held", bundleObservation);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we generate a separate images.csv for held images. This works for the scconfigHeld test, but not for FunctionalTestJigsawHeldList I haven't figured out why yet.

for (int i = 0; i < size(); i++) {
numParameters += at(i)->numberParameters();
}
return numParameters;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kberryUSGS There's a chance you already made this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this a bit differently locally, but your version is better, taken along with your other changes.

EXPECT_NEAR(elems.at(56).toDouble(), 0.365064224, 0.00001);


QFile rightFile(prefix.path() + "/bundleout_images_LUNARRECONNAISSANCEORBITER_NACR.csv");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is an example of how the new images.csv output could be annoying. The NACL and NACR are two copies of the same system and we would solve for their images exactly the same way, but because they are actually different instrument IDs, we will output two images.csv files when we don't need to.

I want to talk to some users to see how they feel about this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a great idea. If it does turn out that the users don't like this, is there a straightforward way to produce just one output CSV in the case where the solve parameters are identical?

Copy link
Contributor

@krlberry krlberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Some comments / questions.

EXPECT_NEAR(elems.at(56).toDouble(), 0.365064224, 0.00001);


QFile rightFile(prefix.path() + "/bundleout_images_LUNARRECONNAISSANCEORBITER_NACR.csv");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a great idea. If it does turn out that the users don't like this, is there a straightforward way to produce just one output CSV in the case where the solve parameters are identical?

for (int i = 0; i < size(); i++) {
numParameters += at(i)->numberParameters();
}
return numParameters;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this a bit differently locally, but your version is better, taken along with your other changes.

@jessemapel jessemapel requested a review from krlberry May 12, 2021 19:27
@krlberry krlberry merged commit ad4bc71 into DOI-USGS:csmbundle May 12, 2021
amystamile-usgs pushed a commit to amystamile-usgs/ISIS3 that referenced this pull request May 12, 2021
jessemapel pushed a commit that referenced this pull request May 13, 2021
* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR #4446 changes & removed makefile tests

* Updates to scconfig tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 8, 2021
* CSM CSV output

* Small test clean-up

* typo fix

* obs header fix

* Remove param list from bo

* Added debug output

* Now reurning

* Fixed unit test

* xml save test update

* Clean up bundleresults unit test output

* Fixed qfile member

* Fixed old path

* Changed header to use param names

* Making separate csvs

* Added reverse on obs by instId

* Trying to fix multiple sensors

* Fixed images header

* More debug

* removed debug outs

* Added terrible imageindex search

* Added held config fix

* Updated for observation test

* Updated jigsaw test makefiles

* Updated with new tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 8, 2021
* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR DOI-USGS#4446 changes & removed makefile tests

* Updates to scconfig tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 8, 2021
* CSM CSV output

* Small test clean-up

* typo fix

* obs header fix

* Remove param list from bo

* Added debug output

* Now reurning

* Fixed unit test

* xml save test update

* Clean up bundleresults unit test output

* Fixed qfile member

* Fixed old path

* Changed header to use param names

* Making separate csvs

* Added reverse on obs by instId

* Trying to fix multiple sensors

* Fixed images header

* More debug

* removed debug outs

* Added terrible imageindex search

* Added held config fix

* Updated for observation test

* Updated jigsaw test makefiles

* Updated with new tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 8, 2021
* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR DOI-USGS#4446 changes & removed makefile tests

* Updates to scconfig tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 8, 2021
* CSM CSV output

* Small test clean-up

* typo fix

* obs header fix

* Remove param list from bo

* Added debug output

* Now reurning

* Fixed unit test

* xml save test update

* Clean up bundleresults unit test output

* Fixed qfile member

* Fixed old path

* Changed header to use param names

* Making separate csvs

* Added reverse on obs by instId

* Trying to fix multiple sensors

* Fixed images header

* More debug

* removed debug outs

* Added terrible imageindex search

* Added held config fix

* Updated for observation test

* Updated jigsaw test makefiles

* Updated with new tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 8, 2021
* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR DOI-USGS#4446 changes & removed makefile tests

* Updates to scconfig tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 9, 2021
* CSM CSV output

* Small test clean-up

* typo fix

* obs header fix

* Remove param list from bo

* Added debug output

* Now reurning

* Fixed unit test

* xml save test update

* Clean up bundleresults unit test output

* Fixed qfile member

* Fixed old path

* Changed header to use param names

* Making separate csvs

* Added reverse on obs by instId

* Trying to fix multiple sensors

* Fixed images header

* More debug

* removed debug outs

* Added terrible imageindex search

* Added held config fix

* Updated for observation test

* Updated jigsaw test makefiles

* Updated with new tests
tgiroux pushed a commit to tgiroux/ISIS3 that referenced this pull request Jun 9, 2021
* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR DOI-USGS#4446 changes & removed makefile tests

* Updates to scconfig tests
jessemapel added a commit that referenced this pull request Jun 17, 2021
* CSM CSV output

* Small test clean-up

* typo fix

* obs header fix

* Remove param list from bo

* Added debug output

* Now reurning

* Fixed unit test

* xml save test update

* Clean up bundleresults unit test output

* Fixed qfile member

* Fixed old path

* Changed header to use param names

* Making separate csvs

* Added reverse on obs by instId

* Trying to fix multiple sensors

* Fixed images header

* More debug

* removed debug outs

* Added terrible imageindex search

* Added held config fix

* Updated for observation test

* Updated jigsaw test makefiles

* Updated with new tests
jessemapel pushed a commit that referenced this pull request Jun 17, 2021
* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR #4446 changes & removed makefile tests

* Updates to scconfig tests
jessemapel added a commit that referenced this pull request Jun 29, 2021
* copylab

* Initial work toward a working AbstractBundleObservation class

* Fix accidentl add of temporary changes from another project

* Trial 2 on fixing my accidental changes

* csm bundle building properly (#4409)

* Updates for linking and compiling

* Starting virtual functions

* Add virtuals to AbstractBundleObservation and get compiling again

* Remove function from parent

* Get jigsaw tests passing on csmbundle branch (#4413)

* Add virtuals to AbstractBundleObservation and get compiling again

* Remove function from parent

* Get existing jigsaw tests passing

* Remove unneeded functions from BundleObservation

* Clean up a bit more

* Clean up BundleObservation and AbstractBundleObservation more

* Remove unneeded BodyRotation

* Add target body solve back into bundle adjusment calc

* Remove debug output

* Removed unneeded memeber variables BundleObservation

Co-authored-by: Kristin Berry <kberry@gyro.wr.usgs.gov>

* Move some functions to pure virtual

* Csmobservation (#4419)

* Now compiling

* Updated CSMCamera tests and docs

* Remove duplicated member variables and accesor functions from BundleObservation and add Csm camera type to Camera enum

* Remove commented-out code

* Added required headers from removed ground map header (#4427)

* last missing header (#4428)

* Moved CSM settings into BOSS

* Generates Serial Numbers using CSM label information (#4437)

* Creates Serial Numbers using CSM label information

* Updated second SerialNumberList:add function to accommodate CSM label

* Add hooks for csm bundle settings to jigsaw (#4438)

* In progress

* Added tests and Pvl support

* Add missing 'y' to docs

Co-authored-by: Kristin Berry <kberry@usgs.gov>

* Moves partial calculation into CSM and ISIS observation classes. (#4436)

* Remove duplicated member variables and accesor functions from BundleObservation and add Csm camera type to Camera enum

* Remove commented-out code

* Imagecoeff migrated into BundleObservation class working

* Get base level working for all coeffs in ISIS

* Fix accidental move into if statement to get tests passing again

* Updated RHS for CsmBundleObservation to work

* Adds point3D support to CSM Camera observations and also adds ground Partials member function to CSMCamera

* Draft of sensor partials code

* Add docs, some fixes based on review requests

* move computation of weights and observation values to bundle observations

* Add observationvalue function to CsmBundleObservation and move the weighting that it is possible to to the observation classes

* Make CSMSolveSet an optional parameter to get jigsaw tests passing again

* cleanup

* Move observation weights into MLE if block

Co-authored-by: Kristin Berry <kberry@gyro.wr.usgs.gov>

* Implemented bundleOutputString for CSM (#4444)

* Added test for CSM bundleOutputString

* Passing unit test and updates to bundleOutputString

* Addressed PR feedback

* Csmcsv (#4446)

* CSM CSV output

* Small test clean-up

* typo fix

* obs header fix

* Remove param list from bo

* Added debug output

* Now reurning

* Fixed unit test

* xml save test update

* Clean up bundleresults unit test output

* Fixed qfile member

* Fixed old path

* Changed header to use param names

* Making separate csvs

* Added reverse on obs by instId

* Trying to fix multiple sensors

* Fixed images header

* More debug

* removed debug outs

* Added terrible imageindex search

* Added held config fix

* Updated for observation test

* Updated jigsaw test makefiles

* Updated with new tests

* Updates Bundle Observation Vector to support Csm observations. (#4457)

* Remove duplicated member variables and accesor functions from BundleObservation and add Csm camera type to Camera enum

* Remove commented-out code

* Imagecoeff migrated into BundleObservation class working

* Get base level working for all coeffs in ISIS

* Fix accidental move into if statement to get tests passing again

* Updated RHS for CsmBundleObservation to work

* Adds point3D support to CSM Camera observations and also adds ground Partials member function to CSMCamera

* Draft of sensor partials code

* Add docs, some fixes based on review requests

* move computation of weights and observation values to bundle observations

* Add observationvalue function to CsmBundleObservation and move the weighting that it is possible to to the observation classes

* Make CSMSolveSet an optional parameter to get jigsaw tests passing again

* cleanup

* Move observation weights into MLE if block

* Added isis vs. csm support to Bundle Observation Vector class to allow jigsaw to proceed past the setup stage for csm networks

* Comment cleanup

* Fix failing unit tests

* Add docs

* Updated based on feedback with potentially temporary fix to get unit tests passing

* Update tests a bit

* Aligned signature definitions and cleaned up

* remove unneeded functions

* More comment cleanup

Co-authored-by: Kristin Berry <kberry@gyro.wr.usgs.gov>

* Jigsaw Test Conversions (sccconfig, scconfigHeld, radar) (#4463)

* Converted jigsaw scconfig tests.

* Added jigsaw radar test

* Updated tests to reflect PR #4446 changes & removed makefile tests

* Updates to scconfig tests

* Gets csm bundle converging (#4464)

* Added cam type checks

* checking csm partials

* Debug partials

* debug corrections

* Fixed index

* Scaled ground partials

* Updated param comparison

* Removed debug out

* Fixes spacing issue in CSM bundleout.txt (#4465)

* Update csm bundlout to have correct spacing for paramter names

* Reverted unintential change

Co-authored-by: Kristin Berry <kberry@gyro.wr.usgs.gov>

* Removed unit-test specific changes I made to the code instead of the unit test

* Add latitudinal partials for CSM (#4468)

* Add latitudinal partials for CSM

* Added test

Co-authored-by: Jesse Mapel <jmapel@usgs.gov>

* Csm clean up (#4483)

* Added cms index list error

* Added CSMCameraTests for partials

* Observation clean-up

* Cleaned up vector

* Rename BundleObservation classes (#4518)

* rename BundleObservation classes

* omit changes to sip wrappers

* Adds more capabilities to TestCsmModel (#4527)

* Updated to get new isd/state form to test jigsaw parsing

* Add functional image-to-ground, ground-to-image, and isd-parsing to TestCsmModel

* Remove temporary csminit test changes

* Updated variable name for clarity

* add csm info to app docs for csminit and jigsaw

* Csmbundle - remove SIP wrappers and deprecated formatBundleOutputString (#4526)

* remove sip wrappers

* remove deprecated formatBundleOutputString

* remove more sip code and formatBundleOutputString

* Fix functional tests to work with updated TestCsmModel (#4534)

* Tests + test data for CSM jigsaw (#4530)

* Removed debug couts

* Added csmnetwork test fixture

* Added a test for CSM test network

* Adjusted trig functions + added deg/rad conversions

* Initial CSMNetwork data

* Uncommented code

* Removed network path and control network from CSMNetwork fixture

* Removed temporary path from csv comparison

* Adjusted spacing + responded to PR feedback

Co-authored-by: Austin Sanders <arsanders@ugs.gov>

* Added the ability to update CSM model state in jigsaw (#4529)

* Added the ability to update CSM model state in jigsaw

* Added missing function docs

* Updated CSM jigsaw test to check apply

* Wrangled up an F

* Added changelog entry

* General clean-up

* Changed Camera local photometric calculations to not use bodyrotation

* Changed csm camera to save look on non-intersection

* Fixed docs typos

Co-authored-by: Kristin Berry <kberry@gyro.wr.usgs.gov>
Co-authored-by: Kristin Berry <kberry@usgs.gov>
Co-authored-by: Amy Stamile <74275278+amystamile-usgs@users.noreply.github.com>
Co-authored-by: Tim Giroux <62255438+tgiroux@users.noreply.github.com>
Co-authored-by: Tim Giroux <tgiroux@contractor.usgs.gov>
Co-authored-by: AustinSanders <arsanders@usgs.gov>
Co-authored-by: Austin Sanders <arsanders@ugs.gov>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants