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

PERF: Applying patch suggested by OTB mantis issue #257 #5

Closed
wants to merge 2 commits into from
Closed

PERF: Applying patch suggested by OTB mantis issue #257 #5

wants to merge 2 commits into from

Conversation

jmichel-otb
Copy link
Contributor

https://bugs.orfeo-toolbox.org/view.php?id=257

Hi all,

When dealing with sensor models (in my case a TerraSar-X sensor
model), I've noticed that the application of a forward projection
(image to world coordinates) followed by an inverse projection (world
to image coordinates) leads to an unexpected result... that is, we
don't land on our feet ! The resulting image coordinates are (in my
case) 10 pixels far from the initial image coordinates.

In the method ossimSensorModel::worldToLineSample, the method
lineSampleHeightToWorld is iteratively called with a fixed height (the
height of the world point given as argument of
ossimSensorModel::worldToLineSample) what I think is not correct.
I've run tests where I replaced calls to lineSampleHeightToWorld with
calls to lineSampleToWorld (lines 346, 347 & 348 of file
ossimSensorModel.cpp) and the results are much better (the resulting
image coordinates are less than 0.1 pixel far from the initial ones).

The call to lineSampleHeightToWorld could be a voluntary choice made
by the software developer in order to have a less time-consuming
method (?). Indeed, lineSampleToWorld implements an iterative
intersection between a ray and the DEM whereas lineSampleHeightToWorld
does not.
However, I think it's worth it because the location accuracy (from
world to image) would be (greatly) improved.

You can find the suggested modification in attached file.

Thank you for your answers,

Best regards,

Edouard

https://bugs.orfeo-toolbox.org/view.php?id=257

Hi all,

When dealing with sensor models (in my case a TerraSar-X sensor
model), I've noticed that the application of a forward projection
(image to world coordinates) followed by an inverse projection (world
to image coordinates) leads to an unexpected result... that is, we
don't land on our feet ! The resulting image coordinates are (in my
case) 10 pixels far from the initial image coordinates.

In the method ossimSensorModel::worldToLineSample, the method
lineSampleHeightToWorld is iteratively called with a fixed height (the
height of the world point given as argument of
ossimSensorModel::worldToLineSample) what I think is not correct.
I've run tests where I replaced calls to lineSampleHeightToWorld with
calls to lineSampleToWorld (lines 346, 347 & 348 of file
ossimSensorModel.cpp) and the results are much better (the resulting
image coordinates are less than 0.1 pixel far from the initial ones).

The call to lineSampleHeightToWorld could be a voluntary choice made
by the software developer in order to have a less time-consuming
method (?). Indeed, lineSampleToWorld implements an iterative
intersection between a ray and the DEM whereas lineSampleHeightToWorld
does not.
However, I think it's worth it because the location accuracy (from
world to image) would be (greatly) improved.

You can find the suggested modification in attached file.

Thank you for your answers,

Best regards,

Edouard
@oscarkramer
Copy link
Member

Salut Eduoard,

I agree with Garrett that it appears this problem is unique to ossimTerraSarModel/ossimGeometricSarSensorModel. I don't think you should put that change in ossimSensorModel as that would affect all sensor models, including those that work fine for the round-trip test you did. I suggest overriding worldToLineSample in ossimGeometricSarSensorModel and putting your changes to use the iterative lineSampleToWorld() calls in that implementation. So only your model would be affected. Better yet, perhaps you could try and figure out why the base-class implementation is not working. It is very curious that the round-trip is failing. Perhaps there is some numerical instability and the max number of iterations is reached. Maybe there is a discontinuity in the DEM at that location? A null post? Let us know what you find.

Also, as Garrett pointed out, you have the wrong arguments in the lineSampleToWorld() calls. Please make sure everything compiles first! You should also run your tests before doing the pull request.

Finally, do a "git rebase dev" from your branch as that is the correct branch to use for development.

Oscar

@jmichel-otb
Copy link
Contributor Author

Hi,

I am sorry for this innacurate patch. In fact my name is not Edouard, I am
Julien Michel from the Orfeo ToolBox, and I was cleaning up our bugtracker
when I found this quite old issue (reported by a guy named Edouard) :

https://bugs.orfeo-toolbox.org/view.php?id=257

This is a patch for ossim, not OTB. It is old but I thought it was worth
sharing it with you instead of discarding it. I had to redo the changes
myself since the patch was based on an old version of OSSIM, and true, I
did not compile it (because my main purpose for today was cleaning up the
bugtracker, not making clean pull requests to OSSIM).

It is perfectly fine with me if you chose to discard the changes, or
implement them elsewhere. I did a pull request because if the patch is
really worth something, it is less pain for you to merge it.

Sorry if that lacked details for complete understanding ...

Regards,

Julien

2015-11-10 14:34 GMT+01:00 Oscar Kramer notifications@github.com:

Salut Eduoard,

I agree with Garrett that it appears this problem is unique to
ossimTerraSarModel/ossimGeometricSarSensorModel. I don't think you should
put that change in ossimSensorModel as that would affect all sensor
models, including those that work fine for the round-trip test you did. I
suggest overriding worldToLineSample in ossimGeometricSarSensorModel and
putting your changes to use the iterative lineSampleToWorld() calls in that
implementation. So only your model would be affected. Better yet, perhaps
you could try and figure out why the base-class imleentation is not
working. It is very curious that the round-trip is failing. Perhaps there
is some numerical instability and the max number of iterations is reached.
Maybe there is a discontinuity in the DEM at that location? A null post?
Let us know what you find.

Also, as Garrett pointed out, you have the wrong arguments in the
lineSampleToWorld() calls. Please make sure everything compiles first! You
should also run your tests before doing the pull request.

Finally, do a "git rebase dev" from your branch as that is the correct
branch to use for development.

Oscar


Reply to this email directly or view it on GitHub
#5 (comment).

@gpotts
Copy link
Member

gpotts commented Dec 17, 2015

This patch is way too old and need to be rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants