-
Notifications
You must be signed in to change notification settings - Fork 168
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
New backplane options for SunIllumination and SurfaceObliqueDetectorResolution have been added to phocube #5488
New backplane options for SunIllumination and SurfaceObliqueDetectorResolution have been added to phocube #5488
Conversation
phocube * Added robust implementation to determine solar illumination on body considering terrain occlusion * Added app test for this option * Updated documentation describing this new feature
* phocube - This new plane is in addition to the existing ObliqueDetectorResolution. The difference is ObliqueDetectorResolution uses the local (shape model) emission angle and SurfaceObliqueDetectorResolution uses the emission angle calculated from the ellipsoid. Signed-off-by: Kris J. Becker <kbecker@orex.lpl.arizona.edu>
…EDETECTORRESOLUTION backplanes. Added gtests for these options. Updated documentation and added an example. Addresses DOI-USGS#5467.
…d via cherry-pick from U. of Arizona codebase. Addresses DOI-USGS#5467.
…ANGELOG.md for phocube backplane additions. Addresses DOI-USGS#5467.
For what it's worth I have run the entire test suite on this branch (OSX 12.7. 5 Monterey), comparing to ISIS dev as a baseline (also run on OSX). Test failures were identical between the two and are shown below... Module Test Failures App Test Failures Unit Test Failures Other Test Failures |
Unable to run tests due to merge conflicts. |
Working on it. Thanks for having a look.
…On Mon, Jul 15, 2024 at 3:04 PM Amy Stamile ***@***.***> wrote:
Unable to run tests due to merge conflicts.
—
Reply to this email directly, view it on GitHub
<#5488 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABUGSURDSJO26WVREVLKKJ3ZMRBPVAVCNFSM6AAAAABG3FZYECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRZGUYTMMRXGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Tests passing for this PR. Ready for a review. @acpaquette Do you have time to look at this PR? |
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.
Overall looks good, couple things to address then this can get merged
double emission = cam->EmissionAngle() * DEG2RAD; | ||
if ( emission < HALFPI ) { | ||
out[index] = cam->DetectorResolution() / cos(emission); | ||
} |
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.
Can this be replaced with the following?
out[index] = cam->ObliqueDetectorResolution(useLocal=false);
This should be the original math before it was changed to default to use the local emission
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.
@kledmundson Apologies for any confusion, the set of 4 lines I highlighted here can be replaced with the ObliqueDetectorResolution
call. So this:
double emission = cam->EmissionAngle() * DEG2RAD;
if ( emission < HALFPI ) {
out[index] = cam->ObliqueDetectorResolution(false) / cos(emission);
}
Should become this:
out[index] = cam->ObliqueDetectorResolution(false);
As the code in the camera function looks like this:
if(!HasSurfaceIntersection()) {
return Isis::Null;
}
double thetaRad;
double emissionDeg;
if(useLocal) {
Angle phase, emission, incidence;
bool success;
LocalPhotometricAngles(phase, incidence, emission, success);
emissionDeg = (success) ? emission.degrees() : Isis::Null;
}
else {
emissionDeg = EmissionAngle();
}
thetaRad = emissionDeg*DEG2RAD;
if (thetaRad < HALFPI) {
return DetectorResolution()/cos(thetaRad);
}
return Isis::Null;
So if we run with useLocal
set to false we run the following:
if(!HasSurfaceIntersection()) {
return Isis::Null;
}
double thetaRad;
double emissionDeg;
emissionDeg = EmissionAngle();
thetaRad = emissionDeg*DEG2RAD;
if (thetaRad < HALFPI) {
return DetectorResolution()/cos(thetaRad);
}
return Isis::Null;
On it. Thanks a lot.
…On Tue, Jul 23, 2024 at 1:08 PM acpaquette ***@***.***> wrote:
***@***.**** commented on this pull request.
Overall looks good, couple things to address then this can get merged
------------------------------
In isis/src/base/apps/phocube/phocube.cpp
<#5488 (comment)>:
> + double emission = cam->EmissionAngle() * DEG2RAD;
+ if ( emission < HALFPI ) {
+ out[index] = cam->DetectorResolution() / cos(emission);
+ }
Can this be replaced with the following?
out[index] = cam->ObliqueDetectorResolution(useLocal=false);
This should be the original math before it was changed to default to use
the local emission
------------------------------
In isis/src/base/apps/phocube/phocube.xml
<#5488 (comment)>:
> @@ -898,7 +979,7 @@ xsi:noNamespaceSchemaLocation=
FROM
</parameterName>
</image>
- </inputImages>
+ </inputImages>S
Errant S?
—
Reply to this email directly, view it on GitHub
<#5488 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABUGSURD2XFP5UJB5LTUJSTZN2Z5HAVCNFSM6AAAAABG3FZYECVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJUHE3DMNJZGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
…->ObliqueDetectorResolution(false), where "false" refers to the useLocal parameter; and 2) fixed typo in phocube.xml. Addresses DOI-USGS#5467.
…olution(false)" in phocube.cpp. Addresses
Sorry, brain lock on my part. Hopefully sorted out now.
…On Tue, Jul 23, 2024 at 4:03 PM acpaquette ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In isis/src/base/apps/phocube/phocube.cpp
<#5488 (comment)>:
> + double emission = cam->EmissionAngle() * DEG2RAD;
+ if ( emission < HALFPI ) {
+ out[index] = cam->DetectorResolution() / cos(emission);
+ }
@kledmundson <https://github.com/kledmundson> Apologies for any
confusion, the set of 4 lines I highlighted here can be replaced with the
ObliqueDetectorResolution call. So this:
double emission = cam->EmissionAngle() * DEG2RAD;if ( emission < HALFPI ) {
out[index] = cam->ObliqueDetectorResolution(false) / cos(emission);
}
Should become this:
out[index] = cam->ObliqueDetectorResolution(false);
As the code in the camera function looks like this:
if(!HasSurfaceIntersection()) {
return Isis::Null;
}
double thetaRad;
double emissionDeg;
if(useLocal) {
Angle phase, emission, incidence;
bool success;
LocalPhotometricAngles(phase, incidence, emission, success);
emissionDeg = (success) ? emission.degrees() : Isis::Null;
}
else {
emissionDeg = EmissionAngle();
}
thetaRad = emissionDeg*DEG2RAD;
if (thetaRad < HALFPI) {
return DetectorResolution()/cos(thetaRad);
}
return Isis::Null;
So if we run with useLocal set to false we run the following:
if(!HasSurfaceIntersection()) {
return Isis::Null;
}
double thetaRad;
double emissionDeg;
emissionDeg = EmissionAngle();
thetaRad = emissionDeg*DEG2RAD;
if (thetaRad < HALFPI) {
return DetectorResolution()/cos(thetaRad);
}
return Isis::Null;
—
Reply to this email directly, view it on GitHub
<#5488 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABUGSUTOHZTGFBNURJDYU73ZN3ONVAVCNFSM6AAAAABG3FZYECVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJVGIZDENJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Actually, now this test is failing... FunctionalTestPhocubeSurfaceObliqueDetectorResolution Looking at it now. |
Tests look good with recent changes. |
Description
SunIllumination and SurfaceObliqueDetectorResolution backplane options have been added to the phocube application. Documentation (including an example) and gtests have been added to address these additions. A small (118K) cube has been added to ..isis/tests/data/phocube to support the new gtests. The University of Arizona OSIRIS-REx Image Processing group have added these additional backplanes to their version of phocube.
The SunIlluminationMask (or shadow mask), is particularly useful for small irregular bodies (e.g. comets, asteroids) where terrain may be occluded from solar illumination at the surface intersection point at a given pixel. A pixel in the SunIlluminationMask backplane is either 0 (occluded) or 1 (illuminated), as determined by ray tracing. The existing Oblique Detector Resolution backplane is computed using the local emission angle from the shape model whereas the new Surface Oblique Detector Resolution backplane is computed using the emission angle as determined from the ellipsoid, producing a smoother data plane.
Related Issue
#5467
How Has This Been Validated?
Results of ctest suite...
ctest -R Phocube --output-on-failure
Test project /Users/kledmundson/ISISDev/phocube/Apr082024/ISIS3/build
Start 1493: DefaultCube.FunctionalTestPhocubeDefault
1/11 Test #1493: DefaultCube.FunctionalTestPhocubeDefault ........................ Passed 1.87 sec
Start 1494: DefaultCube.FunctionalTestPhocubeAllBands
2/11 Test #1494: DefaultCube.FunctionalTestPhocubeAllBands ....................... Passed 4.40 sec
Start 1495: DefaultCube.FunctionalTestPhocubeSpecialPixels
3/11 Test #1495: DefaultCube.FunctionalTestPhocubeSpecialPixels .................. Passed 1.74 sec
Start 1496: DefaultCube.FunctionalTestPhocubeMosaic
4/11 Test #1496: DefaultCube.FunctionalTestPhocubeMosaic ......................... Passed 1.74 sec
Start 1497: DefaultCube.FunctionalTestPhocubeNoBandBin
5/11 Test #1497: DefaultCube.FunctionalTestPhocubeNoBandBin ...................... Passed 1.86 sec
Start 1498: DefaultCube.FunctionalTestPhocubeAllDnBands
6/11 Test #1498: DefaultCube.FunctionalTestPhocubeAllDnBands ..................... Passed 1.85 sec
Start 1775: TempTestingFiles.UnitTestMarciCameraPhocubeBandChange
7/11 Test #1775: TempTestingFiles.UnitTestMarciCameraPhocubeBandChange ........... Passed 0.85 sec
Start 2082: OffBodyCube.FunctionalTestPhocubeOffBody
8/11 Test #2082: OffBodyCube.FunctionalTestPhocubeOffBody ........................ Passed 0.90 sec
Start 2083: MiniRFCube.FunctionalTestPhocubeMiniRF
9/11 Test #2083: MiniRFCube.FunctionalTestPhocubeMiniRF .......................... Passed 0.92 sec
Start 2084: Phocube.FunctionalTestPhocubeSunIlluminationMask
10/11 Test #2084: Phocube.FunctionalTestPhocubeSunIlluminationMask ................ Passed 7.95 sec
Start 2085: Phocube.FunctionalTestPhocubeSurfaceObliqueDetectorResolution
11/11 Test #2085: Phocube.FunctionalTestPhocubeSurfaceObliqueDetectorResolution ... Passed 6.83 sec
100% tests passed, 0 tests failed out of 11
Total Test time (real) = 31.11 sec
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: