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.

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

Errors for MEX HRSC images using DSK shape model and isis ray tracing engine #5036

Open
lwellerastro opened this issue Aug 18, 2022 · 13 comments
Labels
bug Something isn't working inactive Issue that has been inactive for at least 6 months Products Issues which are impacting the products group

Comments

@lwellerastro
Copy link
Contributor

ISIS version(s) affected: isis7.1.0-RC1

Description
Programs such as camstats, caminfo, qview throw errors for MEX HRSC images initialized with DSK shapemodel:

    Program = camstats
    Class   = "PROGRAMMER ERROR"
    Code    = 3
    Message = "Sensor::SetGround failed for surface point in
               LineScanCameraGroundMap.cpp LineOffsetFunctor"
    File    = LineScanCameraGroundMap.cpp
    Line    = 86

I do not encounter these problems under this version when working with MEX SRC DSK initialized images (same DSK as HRSC and using isis ray tracing).

How to reproduce
An image is available under my user work area /Isis3Tests/DSK. It was processed in the following manner:

hrsc2isis from=H0756_0000_P22.IMG to=H0756_0000_P22.cub
spiceinit from=H0756_0000_P22.cub shape=user model=PHOBOS_K275_DLR_V02.BDS
camstats from=H0756_0000_P22.cub to=cstats.pvl linc=10 sinc=10

Additional context
Errors are occurring under this version of ISIS for images that had no such errors under isis7.0.0 using the default isis ray tracing engine.

If I point to a preference file to use bullet while running camstats, the program runs without error:
camstats from=H0756_0000_P22.cub to=cstats.pvl linc=10 sinc=10 -preference=IsisPreferences_Bullet

I chose to default to isis ray tracing under 7.1.0-RC1 for testing because I had more success using it under isis7.0.0 for programs like footprintinit and caminfo polygon (bullet had many more failures for those programs for reasons I don't understand).

I ran camstats on >1000 HRSC images (originally processed under isis7.0.0) and >800 had the failure indicated above. Let me know if you would like to see more failure images or any successful ones.

@lwellerastro lwellerastro added bug Something isn't working Products Issues which are impacting the products group labels Aug 18, 2022
@Kelvinrr
Copy link
Collaborator

I wasn't able to reproduce so I think this was solved along with #5049

re-open if it continues to be a problem.

@lwellerastro
Copy link
Contributor Author

lwellerastro commented Sep 19, 2022

This problem preceded the new data area and has nothing to do with #5049 post. The failure in the original post still exists.

In order to reproduce properly you must use the isis ray tracing engine for every program. If bullet is turned on it will work.

Isis ray tracing works in isis7.0.0, but not under 7.1.0-RC1. I think changes made for SRC either via #4803 or more likely #4943 didn't account for HRSC or maybe broke something that had been working.

@github-actions
Copy link

Thank you for your contribution!

Unfortunately, this issue hasn't received much attention lately, so it is labeled as 'stale.'

If no additional action is taken, this issue will be automatically closed in 180 days.

@github-actions github-actions bot added the inactive Issue that has been inactive for at least 6 months label Mar 29, 2023
@lwellerastro
Copy link
Contributor Author

lwellerastro commented Mar 29, 2023

This problem still exists and now under isis7.2.0. See my work Isis3Tests/DSK/isis7.2.0/ directory for data and proc.scr for details on how to reproduce this there.

This is not high priority.

I'm unable to remove the inactive label for some reason. In fact it looks like I can no longer add or modify label information any longer for some reason.

@github-actions github-actions bot removed the inactive Issue that has been inactive for at least 6 months label Mar 30, 2023
@chkim-usgs chkim-usgs self-assigned this May 1, 2023
@chkim-usgs
Copy link
Contributor

Hi @lwellerastro, I believe you are correct in that the solution for #4943 resulted in this issue. This was the added intersectSurface() function (also found here):

   bool NaifDskShape::intersectSurface(const SurfacePoint &surfpt,
                                    const std::vector<double> &observerPos,
                                    const bool &backCheck) {

    std::vector<double> look(3);
    look[0] = surfpt.GetX().kilometers() - observerPos[0];
    look[1] = surfpt.GetY().kilometers() - observerPos[1];
    look[2] = surfpt.GetZ().kilometers() - observerPos[2];

    return intersectSurface(observerPos, look);

  }

I was able to successfully run camstats with the data you provided and default ISIS preferences (no specified RayTraceEngine) by essentially replacing (temporarily) the above function logic with the one in ShapeModel, where the base intersectSurface() function is the default behavior that does not run any intersection tests (here):

  bool ShapeModel::intersectSurface(const SurfacePoint &surfpt, 
                                    const std::vector<double> &observerPos,
                                    const bool &backCheck) {

    // The default behavior is to set the point in the model without
    //  intersection tests at all
    setSurfacePoint(surfpt);
    return (true);
  }

Referring to the original post, the issue occurs with MEX HRSC DSK initialized images but no issue with MEX SRC DSK initialized images. I am planning on using the input cube to extract the value from InstrumentId under Group Instrument so I can set a conditional to use either intersectSurface() logic based on the InstrumentId value (SRC for the existing function in NaifDskShape, HRSC for the function in ShapeModel). Would you know of other InstrumentId values to expect in this scenario (other than SRC and HRSC) and which logic they would possibly use?

Also please feel free to correct me on anything!

@lwellerastro
Copy link
Contributor Author

lwellerastro commented May 4, 2023

Would you know of other InstrumentId values to expect in this scenario (other than SRC and HRSC) and which logic they would possibly use?

Any mission data where a DSK shape file is available might encounter this problem, I suppose. But I honestly don't understand what's happening to make an informed decision and am concerned something else might be broken via a fix. Pure ignorance here, I'm sorry.

@KrisBecker, do you have any thoughts or concerns with the above possible change or insight into what @chkim-usgs is asking? I know you are very busy with your own work, but I can't provide any useful feedback here and I wouldn't want this very low priority issue that I've been working around to impact far more important mission work. Thanks.

Edit - I'll add that isis ray tracing stinks and I'm primarily using bullet now (after some other issues got addressed since this original post), but it seems to me isis ray tracing ought to work for these data, which it had until a change was made.

@KrisBecker
Copy link
Contributor

Hi @lwellerastro, @chkim-usgs...

Its difficult to understand why so many errors are occurring. It seems likely that changes between 7.0.0 and 7.1.0-RC1 are contributing. My implementation has diverged from the current USGS system and I have made changes that may be relevant.

It is important to not make assumptions about the state of the ray tracing object instances such as NAIF/DSK, Bullet and Embrey. This because ISIS geometric intercept operations may use both the PCK defined shape (e.g., spheres, ellipsoids) and rays tracing intercepts of tessellated plates. This behavior is mostly evident in emission and incidence angle calculations. It is best practice to formally issue ray traces for all intersecting geometries because of this. The consequence of this is ray traces are typically costly.

Here are my commit notes:

  • Fix ray tracing engine emission/incidence angles

    • The NAIF DSK, Bullet and Embree ray tracing engines were not computing ellipsoid and local (shape model) emission and incidence angles. This update makes them consistent in their behavior.

    • The bug is actually in ShapeModel::emissionAngle() and ShapeModel::incidenceAngle(). Neither explicitly computes a surface normal vector but tests to see if one is already computed and uses whatever happens to be there. There is no way to determine if the normal is computed from the ellipsoid or the shape model. Shape models can enforce this by reimplementing this method, but only Embree had this method until USGS removed emissionAngle(), thinking this would fix the problem – this actually created the ellipsoid emission angle consistency problem in phocube.

  • Fixed DSK support for regional shape models

    • BulletTargetShape & EmbreeTargetShape - Corrected lat/lon ntercepts in regional, non-global, shape models. Rays eminating from the body origin were not properly being scaled beyond the surface for inverted ray target body intercepts.

    • NaifDskPlateModel Added try/catch exceptions in NAIF intercept functions when ray tracing fails.

  • ISIS/NAIF DSK shape model modifications

    • NaifDskPlateModel - Added intercept(lat,lon) surface ray trace method to determine radius at lat/lon surface point

    • NaifDskShapeModel - Fixed projection error that occurs when creating output projection and newly added use of local emission to compute oblique detector resolution

  • Fix label issue, preserve RayTraceEngine keyword

    • noproj - fixed bug where the Kernels/RayTraceEngine keyword is dropped when noproj creates the output label. This causes problems for Bullet engine configurations that use the .conf or .obj formats. In fact, Bullet was never used, rather defaults to use the NAIF DSK toolkit for geometry calculations after noproj, which currently does not support terrain occlusion tests because its inefficient compared to Bullet and Embree

Note that these modifications may or may not be relevant (largely due to divergent implementations) to what you are experiencing and require extensive changes. They have really improved consistency in our experience.

@acpaquette
Copy link
Collaborator

@KrisBecker Are all of the above changes/fixes in chronological order?

@KrisBecker
Copy link
Contributor

Mostly, but they need not be addressed in that order.

Copy link

Thank you for your contribution!

Unfortunately, this issue hasn't received much attention lately, so it is labeled as 'stale.'

If no additional action is taken, this issue will be automatically closed in 180 days.

@github-actions github-actions bot added the inactive Issue that has been inactive for at least 6 months label Nov 14, 2023
@chkim-usgs
Copy link
Contributor

@KrisBecker, would you be able to open a PR to address this issue?

@github-actions github-actions bot removed the inactive Issue that has been inactive for at least 6 months label Nov 23, 2023
@KrisBecker
Copy link
Contributor

@chkim-usgs this appears to be a significant issue where the problem domain is not yet well understood. I am not in a position at this time to commit a large amount of resources to investigate this problem and submit a PR but I may be able to help

Simply removing or even disabling #4943 doesn't seem to fully address the problem, but it does contribute in some way to the undesirable behavior.

Has the resulting geometric values (at the pixel level) of camstats been compared between DSK and Bullet? Since you already have a surface intercept in camstats it appears this is checking for occlusion? Following the error message it appears the is some kind of check being made on the intersection, but it is not an occlusion check since that is set to false in the call related to the error message. The HRSC being a line scan instrument it is entirely possible adjacent scan lines image the same location on the surface which may cause issues.

I am not at all surprised there are issues with footprintint and the HRSC. We see a significant amount of self-intersecting geometry in the GIS footprints. In those cases, its sometimes best to use the ellipsoid.

Copy link

Thank you for your contribution!

Unfortunately, this issue hasn't received much attention lately, so it is labeled as 'stale.'

If no additional action is taken, this issue will be automatically closed in 180 days.

If you want to participate in our support prioritization meetings or be notified when support sprints are happening, you can sign up the support sprint notification emails here.

Read more about our support processs here

@github-actions github-actions bot added the inactive Issue that has been inactive for at least 6 months label Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working inactive Issue that has been inactive for at least 6 months Products Issues which are impacting the products group
Projects
Development

No branches or pull requests

5 participants