Fix findfeatures matrix inversion issues and improve FastGeom performance #4772
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| Group = FastGeomCommonParameters | ||
| # Defaults used for findfeatures FastGeom Algorithms | ||
|
|
||
| # FastGeomAlgorithm = radial | ||
| FastGeomPoints = 25 | ||
| FastGeomTolerance = 3 | ||
| # GeomType = camera | ||
|
|
||
| FastGeomQuerySampleTolerance = 0 | ||
| FastGeomQueryLineTolerance = 0 | ||
| FastGeomTrainSampleTolerance = 0 | ||
| FastGeomTrainLineTolerance = 0 | ||
|
|
||
| FastGeomDumpMapping = false | ||
| EndGroup = FastGeomCommonParameters | ||
|
|
||
|
|
||
| Group = FastGeomGridParameters | ||
| # FastGeomAlgorithm = grid | ||
|
|
||
| FastGeomGridStartIteration = 0 | ||
| # FastGeomGridStopIteration = 1 to N | ||
| FastGeomGridIterationStep = 1 | ||
|
|
||
| FastGeomTotalGridIterations = 1 | ||
|
|
||
| FastGeomGridSaveAllPoints = false | ||
| EndGroup = FastGeomGridParameters | ||
|
|
||
|
|
||
| Group = FastGeomRadialParameters | ||
| # FastGeomAlgorithm = radial | ||
|
|
||
| FastGeomRadialSegmentLength = 25 | ||
| FastGeomRadialPointCount = 5 | ||
| FastGeomRadialPointFactor = 1.0 | ||
|
|
||
| # FastGeomRadialSegments = N | ||
| EndGroup = FastGeomRadialParameters | ||
| End |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| #!/bin/bash | ||
Kelvinrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #set -o pipefail | ||
| ############################################################################### | ||
| # | ||
| # Applies basic ISIS processing for control preparation | ||
| # | ||
| # Prerequisites: | ||
| # ISIS3 - must have an appropriate version of system initilized | ||
| # | ||
| # Defaults; | ||
| # Current directory set to processing root dir | ||
| # | ||
| # @author 2021-03-02 Kris Becker Original Version | ||
| ############################################################################### | ||
| usage() { echo "Usage: $0 [-0 {invokes dryrun}] [-h] [-v] \ | ||
| [-S {do not run spiceinit}] [-l] [-p] [-e {error list}] \ | ||
| *.IMG | *.cub \ | ||
| " 1>&2; exit 1; } | ||
|
|
||
| dryrun=0 | ||
| verbose=0 | ||
|
|
||
| run_spice=1 # run spiceinit by default, disable with -S | ||
| pwddir='$PWD' | ||
| lev0dir='$PWD/Lev0' | ||
| lev1dir='$PWD/Lev1' | ||
| lev2dir='$PWD/Lev2' | ||
| prtfile="" | ||
| errlist="" | ||
|
|
||
| ################################################################## | ||
| # finesse_isis_command - Adds additional components to ISIS commmand | ||
| # | ||
| # This function will add logging and preference files to an ISIS | ||
| # command that will be executed within this script. | ||
| # | ||
| # Should be called as: | ||
| # newcmd=$(finesse_isis_command "cmd") | ||
| # | ||
| # @author Kris Becker 2018-05-23 | ||
| ################################################################## | ||
| function finesse_isis_command() { | ||
| command="$1" | ||
|
|
||
| # Check for logging | ||
| logger="" | ||
| if [ -n "${prtfile}" ]; then | ||
| logger="-log=${prtfile}" | ||
| else | ||
| logger="-log=/dev/null" | ||
| fi | ||
|
|
||
| echo "${command} ${logger}" | ||
| } | ||
|
|
||
| ################################################################## | ||
| # execute - Executes a command as a subshell | ||
| # | ||
| # This function will execute a command to the users shell and | ||
| # return status. If the variable $dryurn is set, it will only | ||
| # print out the command and not execute | ||
| # | ||
| # @author Kris Becker 2018-05-23 | ||
| ################################################################## | ||
| function execute() { # (command) | ||
| command="$1" | ||
| echo "$command" | ||
| if [ $dryrun -eq 1 ]; then | ||
| status=0 | ||
| else | ||
| eval $command | ||
| status=$? | ||
| fi | ||
| return $status | ||
| } | ||
|
|
||
| ################################################################### | ||
| # MAIN script section | ||
| # Accumulate the parameters | ||
| ################################################################### | ||
| while getopts ":0hvSlpe:" o; do | ||
| case "${o}" in | ||
| 0) | ||
| dryrun=1 | ||
| verbose=1 | ||
| ;; | ||
| h) | ||
| usage | ||
| ;; | ||
| v) | ||
| verbose=1 | ||
| ;; | ||
| S) | ||
| run_spice=0 | ||
| ;; | ||
| l) | ||
| maklog=1 | ||
| ;; | ||
| p) | ||
| prtfile="print.prt" | ||
| ;; | ||
| e) | ||
| errlist="${OPTARG}" | ||
| ;; | ||
| *) | ||
| usage | ||
| ;; | ||
| esac | ||
| done | ||
| shift $((OPTIND-1)) | ||
|
|
||
| declare -a fromfiles=( "$@" ) | ||
|
|
||
| # Check to see if the input file list exists | ||
| # I. Variable definition | ||
| if [ ${#fromfiles[@]} -le 0 ]; then | ||
| echo "" | ||
| echo "ERROR: No files provided!" | ||
| usage | ||
| exit 1 | ||
| fi | ||
|
|
||
|
|
||
| ############################################################################### | ||
| # Main processing loop. Process all give files checking for errors along | ||
| # the way. | ||
| ############################################################################### | ||
|
|
||
| for from in "${fromfiles[@]}"; do | ||
|
|
||
| ifile="${from##*/}" | ||
| base="${ifile%%.*}" | ||
| ext="${ifile#*.}" | ||
|
|
||
| ( | ||
|
|
||
| # If given a cube, it is assumed import and radiometric calibration has | ||
| # been applied | ||
| edrfile="${from}" | ||
| lev0file="${from}" | ||
| lev1file="${from}" | ||
|
|
||
| # Level0 DATA IMPORT processing | ||
| # Run import and apply radiometric calibrated images | ||
| if [ "${ext}" != "cub" ]; then | ||
| execute "mkdir -p ${lev0dir}" | ||
| lev0file="${lev0dir}/${base}.cub" | ||
| cmd="mdis2isis from=${from} to=${lev0file}" | ||
| execute "$(finesse_isis_command "$cmd")" || exit $? | ||
| fi | ||
|
|
||
| # Apply a prior SPICE kernels | ||
| if [[ $run_spice -eq 1 ]]; then | ||
| cmd="spiceinit from=${lev0file}" | ||
| execute "$(finesse_isis_command "$cmd")" || exit $? | ||
| fi | ||
|
|
||
| # Level1 - radiometric calibration processing | ||
| if [ "${ext}" != "cub" ]; then | ||
| execute "mkdir -p ${lev1dir}" | ||
| lev1file="${lev1dir}/${base}.cal.cub" | ||
| cmd="mdiscal from=${lev0file} to=${lev1file}" | ||
| execute "$(finesse_isis_command "$cmd")" || exit $? | ||
| fi | ||
|
|
||
| # Know Your Data! | ||
| cmd="camstats from=${lev1file} attach=true linc=10 sinc=10" | ||
| execute "$(finesse_isis_command "$cmd")" || exit $? | ||
|
|
||
| cmd="footprintinit from=${lev1file} linc=10 sinc=10 maxemission=89 maxincidence=89 increaseprecision=true" | ||
| execute "$(finesse_isis_command "$cmd")" || exit $? | ||
|
|
||
| ) | ||
|
|
||
| # Subshell exist status and testing | ||
| err=$? | ||
| if [ $err != 0 ]; then | ||
| # If errlist file given, append this file to the list | ||
| if [ -n "$errlist" ]; then | ||
| [ -f "$errlist" ] || execute "touch $errlist" | ||
| execute "echo ${from} >> $errlist" | ||
| fi | ||
| echo "Error encountered with file ${from}" | ||
| fi | ||
| done | ||
|
|
||
| exit 0 | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| #!/bin/sh | ||
|
|
||
| /bin/rm -f *.lis *.cub *.net *.png *_Island.* *.csv *.txt *.log print.prt | ||
| /bin/rm -rf ./Lev0 ./Lev1 ./Lev2 ./Phot ./Updated_Lev1 | ||
|
|
||
| exit 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| Group = Mapping | ||
| ProjectionName = Equirectangular | ||
| TargetName = Mercury | ||
| EquatorialRadius = 2440000.0 | ||
| PolarRadius = 2440000.0 | ||
| CenterLatitude = 0.0 | ||
| CenterLongitude = 180.0 | ||
| LatitudeType = Planetocentric | ||
| LongitudeDirection = PositiveEast | ||
| LongitudeDomain = 360 | ||
| PixelResolution = 200 | ||
| End_Group |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| #!/bin/sh | ||
|
|
||
|
|
||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2008_014/EN0108828436M.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2008_014/EN0108828483M.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2008_014/EN0108828488M.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2011_179/EN0217733143M.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2011_179/EN0217733334M.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2011_183/EN0218118182M.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2011_154/EW0215590428G.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2011_183/EW0218075871G.IMG | ||
| wget -P . https://pds-imaging.jpl.nasa.gov/data/messenger/MDIS/MDIS/msgrmds_1001/DATA/2011_183/EW0218118239G.IMG | ||
|
|
||
| exit 0 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| Object = AutoRegistration | ||
| Group = Algorithm | ||
| Name = MaximumCorrelation | ||
| Tolerance = 0.9 | ||
| End_Group | ||
|
|
||
| Group = PatternChip | ||
| Samples = 31 | ||
| Lines = 31 | ||
| End_Group | ||
|
|
||
| Group = SearchChip | ||
| Samples = 51 | ||
| Lines = 51 | ||
| End_Group | ||
|
|
||
| Group = SurfaceModel | ||
| DistanceTolerance = 1.5 | ||
| WindowSize = 7 | ||
| End_Group | ||
| End_Object | ||
| End | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
As per our discussion at the TC meeting. The fact that his required multiple changelog lines I believe indicates that this should have been split up into multiple PRs.
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.
I agree this PR became much larger than I expected or even imagined. However, the scope was identified and explained in issue #4772 and in the context of this PR on Feb 3, 2022. At no time did anyone mention this concern even after the initial commits were made.
Clearly, the review process (you are now the third reviewer) has contributed greatly to the size of this PR. Its also clear it did get much larger than anyone expected. I also learned a lot about this process and will be mindful to take steps to prevent this from occurring again. For one thing, I will not let my PRs linger so long between reviews - definitely did not help at all.
I am grateful for the reviews as they have been very helpful and useful to make this PR better. The unfortunate consequence of this is it also made it much larger. Much of the size was in response to requests for documentation improvements/clarifications, which is always useful.
I am open for suggestions/discussion about how this PR could be broken up or made easier and more efficient somehow. Hindsight would indicate to not address two issues in one PR even if you think they are small and/or related.
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.
I think at this point, it's too late to break up this PR. It has been through the review process and has had pretty much everything already addressed. But something to consider as a policy to avoid it in the future. Something for the ISIS TC.
Once the merge conflicts are resolved and the scripts are removed, I'll approve and merge this