Skip to content
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.

Sign up for GitHub

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

Fixed bug where focal plane measured x,y coordinates were not set if the cam->SetImage call failed, References #2591). #3193

Merged
merged 3 commits into from
Mar 28, 2019
Merged

Fixed bug where focal plane measured x,y coordinates were not set if the cam->SetImage call failed, References #2591). #3193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f7a852
updates to ControlPoint::ComputeApriori
Mar 12, 2019
6271211
Fixed bug where focal plane measured x,y coordinates were not set if …
Mar 28, 2019
d42161e
Merge branch 'dev' of github.com:USGS-Astrogeology/ISIS3 into dev
Mar 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 129 additions & 92 deletions isis/src/control/objs/ControlPoint/ControlPoint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,11 +839,18 @@ namespace Isis {
}


/**
* This method computes the apriori lat/lon for a point. It computes this
* by determining the average lat/lon of all the measures. Note that this
* does not change ignored, or fixed points. Also, it does not
* use unmeasured or ignored measures when computing the lat/lon.
/**
* Computes a priori lat/lon/radius point coordinates by determining the average lat/lon/radius of
* all measures. Note that this does not change ignored, fixed or constrained points.
*
* Also, it does not use unmeasured or ignored measures when computing lat/lon/radius.
*
* (KLE) Note this not a rigorous triangulation considering outliers. A better way would be to...
* a) use e.g. a closest approach algorithm to find intersection of all rays, regardless
* of whether the intersection lies on the surface in question, then;
* b) perform a rigorous triangulation with some sort outlier detection approach, a robust
* estimation technique (perhaps RANSAC)
*
* @internal
* @history 2008-06-18 Tracie Sucharski/Jeannie Walldren,
* Changed error messages for
Expand Down Expand Up @@ -872,114 +879,88 @@ namespace Isis {
* @history 2017-04-25 Debbie A. Cook - change constraint status calls
* to use generic coordinate names (Coord1, Coord2,
* and Coord3).
* @history 2019-03-10 Ken Edmundson - Fixed bug where focal plane measured x,y coordinates were
* not set if the cam->SetImage call failed. Setting the measured
* focal plane coordinates should not depend upon the success of the
* SetImage call (References #2591). Improved error messages.
* Cleaned up code. Added comments above to suggest a more rigorous
* approach to computing a priori point coordinates.
*
* @return Status Success or PointLocked
*/
ControlPoint::Status ControlPoint::ComputeApriori() {
// TODO (KLE): where should call this go? Also, what's the point? The method has no description.
PointModified();

// Don't goof with fixed points. The lat/lon is what it is ... if
// it exists!
// 2013-11-12 KLE I think this check should include points with any
// number of constrained coordinates??? I agree DAC. *** TODO ***
if (GetType() == Fixed) {
if (!aprioriSurfacePoint.Valid()) {
QString msg = "ControlPoint [" + GetId() + "] is a fixed point ";
msg += "and requires an apriori x/y/z";
throw IException(IException::User, msg, _FILEINFO_);
}
// Don't return until after the FocalPlaneMeasures have been set
// return;
// if point is fixed or constrained, ensure valid a priori point coordinates exist
if ( (IsFixed() || IsConstrained()) && !aprioriSurfacePoint.Valid() ) {
QString msg = "In method ControlPoint::ComputeApriori(). ControlPoint [" + GetId() + "] is ";
msg += "fixed or constrained and requires a priori coordinates";
throw IException(IException::User, msg, _FILEINFO_);
}

double xB = 0.0;
double yB = 0.0;
double zB = 0.0;
double r2B = 0.0;
double xB = 0.0; // body-fixed x
double yB = 0.0; // body-fixed y
double zB = 0.0; // body-fixed z
double r2B = 0.0; // radius squared in body-fixed
int goodMeasures = 0;
double pB[3];

// Loop for each measure and compute the sum of the lat/lon/radii
for (int j = 0; j < cubeSerials->size(); j++) {
ControlMeasure *m = GetMeasure(j);

// The comment code was really checking for candidate measures
// Commented out 2011-03-24 by DAC
// if (!m->IsMeasured()) {
// // TODO: How do we deal with unmeasured measures
// }
// else if (m->IsIgnored()) {
// loop over measures to ...
// 1) set focal plane x,y coordinates for all unignored measures;
// 2) sum latitude, longitude, and radius coordinates in preparation for computing a priori
// coordinates by averaging.
for (int i = 0; i < cubeSerials->size(); i++) {
ControlMeasure *m = GetMeasure(i);
if (m->IsIgnored()) {
// TODO: How do we deal with ignored measures
continue;
}
else {
Camera *cam = m->Camera();
if (cam == NULL) {
QString msg = "The Camera must be set prior to calculating apriori";
throw IException(IException::Programmer, msg, _FILEINFO_);
}
if (cam->SetImage(m->GetSample(), m->GetLine())) {
goodMeasures++;
double pB[3];
cam->Coordinate(pB);
xB += pB[0];
yB += pB[1];
zB += pB[2];
r2B += pB[0]*pB[0] + pB[1]*pB[1] + pB[2]*pB[2];

double x = cam->DistortionMap()->UndistortedFocalPlaneX();
double y = cam->DistortionMap()->UndistortedFocalPlaneY();
m->SetFocalPlaneMeasured(x, y);
}
else {
// JAA: Don't stop if we know the lat/lon. The SetImage may fail
// but the FocalPlane measures have been set
if (GetType() == Fixed) {
continue;
}

// TODO: What do we do
// QString msg = "Cannot compute lat/lon/radius (x/y/z) for "
// "ControlPoint [" + GetId() + "], measure [" +
// m->GetCubeSerialNumber() + "]";
// throw IException(IException::User, msg, _FILEINFO_);

// m->SetFocalPlaneMeasured(?,?);
}
Camera *cam = m->Camera();
if (cam == NULL) {
QString cubeSN = m->GetCubeSerialNumber();
QString msg = "in method ControlPoint::ComputeApriori(). Camera has not been set in ";
msg += "measure for cube serial number [" + cubeSN + "], Control Point id ";
msg += "[" + GetId() + "]. Camera must be set prior to calculating a priori coordinates";
throw IException(IException::Programmer, msg, _FILEINFO_);
}
}

// Don't update the apriori x/y/z for fixed points TODO This needs a closer look
if (GetType() == Free ) {
// point can be tagged as "Free" but still have constrained coordinates
// if tagged "Free" we want to compute approximate a priori coordinates
bool setImageSuccess = cam->SetImage(m->GetSample(), m->GetLine());
m->SetFocalPlaneMeasured(cam->DistortionMap()->UndistortedFocalPlaneX(),
cam->DistortionMap()->UndistortedFocalPlaneY());

// TODO: Seems like we should be able to skip this computation if point is fixed or
// constrained in any coordinate. Currently we are always summing coordinates here. We could
// save time by not doing this for fixed or constrained points.
if (setImageSuccess) {
goodMeasures++;
cam->Coordinate(pB);
xB += pB[0];
yB += pB[1];
zB += pB[2];
r2B += pB[0]*pB[0] + pB[1]*pB[1] + pB[2]*pB[2];
}
}
// if point is "Fixed" or otherwise constrained in one, two, or all three
// coordinates, then we use the a priori surface point coordinates that
// have been given via other means (e.g. through qnet or cneteditor)
// 2013-11-12 KLE Is the next check better as "if Fixed or if # of
// constrained coordinates > 1" ???
else if (GetType() == Fixed
|| NumberOfConstrainedCoordinates() == 3
|| IsCoord1Constrained()
|| IsCoord2Constrained()
|| IsCoord3Constrained()) {

// Initialize the adjusted x/y/z to the a priori coordinates
// if point is Fixed or Constrained in any number of coordinates, initialize adjusted surface
// point to a priori coordinates (set in e.g. qnet or cneteditor) and exit
if( IsFixed() || IsConstrained()) {
adjustedSurfacePoint = aprioriSurfacePoint;

return Success;
}

// Beyond this point, we assume the point is free ***TODO*** confirm this
// Did we have any measures?
// if point is Free, we continue to compute a priori coordinates

// if no good measures, we're done
// TODO: is the message true/meaningful?
if (goodMeasures == 0) {
QString msg = "ControlPoint [" + GetId() + "] has no measures which "
"project to lat/lon/radius (x/y/z)";
QString msg = "in method ControlPoint::ComputeApriori(). ControlPoint [" + GetId() + "] has ";
msg += "no measures which project to the body";
throw IException(IException::User, msg, _FILEINFO_);
}

// Compute the averages if all coordinates are free
//if (NumberOfConstrainedCoordinates() == 0) {
// TODO: confirm if this "if" statement is necessary
if (GetType() == Free || NumberOfConstrainedCoordinates() == 0) {
double avgX = xB / goodMeasures;
double avgY = yB / goodMeasures;
Expand Down Expand Up @@ -1553,11 +1534,6 @@ namespace Isis {
}


bool ControlPoint::IsFixed() const {
return (type == Fixed);
}


SurfacePoint ControlPoint::GetAprioriSurfacePoint() const {
return aprioriSurfacePoint;
}
Expand All @@ -1581,22 +1557,78 @@ namespace Isis {
}


/**
* Return bool indicating if point is Free or not.
*
* @return bool indicating if point is Free or not.
*/
bool ControlPoint::IsFree() const {
return (type != Fixed && type != Constrained);
}


/**
* Return bool indicating if point is Fixed or not.
*
* @return bool indicating if point is Fixed or not.
*/
bool ControlPoint::IsFixed() const {
return (type == Fixed);
}


/**
* Return bool indicating if point is Constrained or not.
*
* @return bool indicating if point is Constrained or not.
*/
bool ControlPoint::IsConstrained() {
// if point type is Free, we ignore any a priori sigmas on the coordinates
if (type == Free) {
return false;
}

return constraintStatus.any();
}


/**
* Return bool indicating if 1st coordinate is Constrained or not. This corresponds to Latitude
* for a Latitudinal solution or X for a Rectangular solution.
*
* @return bool indicating if 1st coordinate is Constrained or not.
*/
bool ControlPoint::IsCoord1Constrained() {
return constraintStatus[Coord1Constrained];
}


/**
* Return bool indicating if 2nd coordinate is Constrained or not. This corresponds to Longitude
* for a Latitudinal solution or Y for a Rectangular solution.
*
* @return bool indicating if 2nd coordinate is Constrained or not.
*/
bool ControlPoint::IsCoord2Constrained() {
return constraintStatus[Coord2Constrained];
}

/**
* Return bool indicating if 3rd coordinate is Constrained or not. This corresponds to Radius
* for a Latitudinal solution or Z for a Rectangular solution.
*
* @return bool indicating if 3rd coordinate is Constrained or not.
*/
bool ControlPoint::IsCoord3Constrained() {
return constraintStatus[Coord3Constrained];
}


/**
* Return bool indicating if point is Constrained or not.
*
* @return bool indicating if point is Constrained or not.
*/
int ControlPoint::NumberOfConstrainedCoordinates() {
return constraintStatus.count();
}
Expand All @@ -1616,6 +1648,7 @@ namespace Isis {
return aprioriRadiusSourceFile;
}


ControlPoint::SurfacePointSource::Source
ControlPoint::GetAprioriSurfacePointSource() const {
return aprioriSurfacePointSource;
Expand Down Expand Up @@ -1990,6 +2023,10 @@ namespace Isis {
}


/**
* What the heck is the point of this?
*
*/
void ControlPoint::PointModified() {
dateTime = "";
}
Expand Down
7 changes: 6 additions & 1 deletion isis/src/control/objs/ControlPoint/ControlPoint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,10 @@ namespace Isis {
* Fixes #5435.
* @history 2018-06-30 Debbie A. Cook Removed all calls to obsolete method
* SurfacePoint::SetRadii. References #5457.
* @history 2019-03-10 Ken Edmundson - See history entry for ComputeApriori method (References
* #2591). Added check to IsConstrained() method to see if point type is
* Free, in which case we ignore stored a priori sigmas on the
* coordinates.
*/
class ControlPoint : public QObject {

Expand Down Expand Up @@ -529,6 +533,7 @@ namespace Isis {
bool IsValid() const;
// Can we get rid of this? It doesn't appear to be used anywhere. *** ToDo ***
bool IsInvalid() const;
bool IsFree() const;
bool IsFixed() const;
bool HasAprioriCoordinates();

Expand Down Expand Up @@ -668,7 +673,7 @@ namespace Isis {
* This stores the constraint status of the a priori SurfacePoint
* @todo Eventually add x, y, and z. Instead we made generic coordinates
*/
std::bitset<6> constraintStatus;
std::bitset<3> constraintStatus;

/**
* This indicates if a program has explicitely set the reference in this
Expand Down