test_shape is broken since OTB 8.2

added bug label

changed the description

By Rémi Cresson on 2023-01-06T09:13:24 (imported from GitLab)

changed the description

By Rémi Cresson on 2023-01-06T09:15:15 (imported from GitLab)

The problem is that after pipeline execution, OTB considers that the Execute() method has to be called again, prior to access the output image parameters?

By Rémi Cresson on 2023-01-06T09:17:55 (imported from GitLab)

Could it be due to UpdateParameters being called sometime when the image is flushed ?

By Vincent Delbar on 2023-01-06T09:20:04 (imported from GitLab)

No. It's the reason I mentioned. See https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb/-/blob/34a0fdbb074480cb859f2166e3ba6b7d1bff555f/Modules/Wrappers/ApplicationEngine/src/otbWrapperApplication.cxx#L952

By Rémi Cresson on 2023-01-06T09:21:16 (imported from GitLab)

the m_ExecuteDone is set back to false after ExecuteAndWriteOutput

By Rémi Cresson on 2023-01-06T09:24:51 (imported from GitLab)

But where ? I see it set to false at the beginning of ExecuteAndWriteOutput, but then Execute is called so it should be set back to true, no ?

By Vincent Delbar on 2023-01-06T09:24:51 (imported from GitLab)

By Rémi Cresson on 2023-01-06T09:28:26 (imported from GitLab)

You are right. I am trying to find out where the set to false occurs.

Methods that set m_ExecuteDone to false:

Application::Application()
Application::UpdateParameters()
Application::PropagateConnectMode()

I don't recall well enough how UpdateParameters and PropagateConnectMode are called in pyotb. Do you?

By Rémi Cresson on 2023-01-06T09:43:21 (imported from GitLab)

I need to check again but it may be due to PropagateConnectMode... but I believe this is not called when we simply init an Input then directly write the result. So UpdateParameters is the main suspect here

By Vincent Delbar on 2023-01-06T09:44:48 (imported from GitLab)

Because UpdateParameters is called when we set the output param during write, so shape may fail after this !

By Vincent Delbar on 2023-01-06T09:47:11 (imported from GitLab)

Still this should not happen with our try except at the end of write():

        try:
            self.app.WriteOutput()
        except RuntimeError:
            logger.debug('%s: failed to simply write output, executing once again then writing', self.name)
            self.app.ExecuteAndWriteOutput()

By Vincent Delbar on 2023-01-06T09:50:04 (imported from GitLab)

Are PropagateConnectMode and UpdateParameters called somewhere from pyotb app write()?

By Rémi Cresson on 2023-01-06T09:51:13 (imported from GitLab)

Propagate no, UpdateParameters yes when we set the output param value (well not us but it is done internally by otb), but then we call ExecuteAndWriteOutput again if needed

By Vincent Delbar on 2023-01-06T09:53:17 (imported from GitLab)

Ok. I feel like this something is not right in the C++ code of OTB. I will investiguate

By Rémi Cresson on 2023-01-06T09:54:24 (imported from GitLab)

Yes. I fear this is the same kind of problem than the previous big one we had : I mean this is occurs when we Execute several times, or Execute then ExecuteAndWriteOutput again. But the following is working (when we write output immediately at init) :

img = pyotb.BandMathX(['tests/image.tif'], exp='im1', out="test.tif")
print(img.shape)

By Vincent Delbar on 2023-01-06T10:03:07 (imported from GitLab)

FYI we completely removed PropagateConnectMode from pyotb ^^
We just use ConnectImage when setting input params

By Vincent Delbar on 2023-01-06T12:38:40 (imported from GitLab)

The following is working, so I really think now that the problem comes from pyotb :/

import otbApplication
img_pth = "/vsicurl/https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb/-/raw/develop/Data/Input/SP67_FR_subset_1.tif?inline=false"

app = otbApplication.Registry.CreateApplication("BandMath")
app.SetParameterStringList("il", [img_pth])
app.SetParameterString("out", "/tmp/toto.tif")
app.SetParameterString("exp", "im1b1")
app.ExecuteAndWriteOutput()  # Can be as well Execute(). If commented, the right exception is thrown
print(app.GetImageSize("out"))

By Rémi Cresson on 2023-01-06T16:18:41 (imported from GitLab)

Okay, I'll try different Execute workflows with this branch

By Vincent Delbar on 2023-01-06T13:01:29 (imported from GitLab)

Hard to say where's the problem since this only failed test is a combination of Input, Slicer and Operation ! + list comprehension

By Vincent Delbar on 2023-01-06T13:31:54 (imported from GitLab)

A code snippet is provided in the issue description to reproduce the bug

By Rémi Cresson on 2023-01-06T14:03:28 (imported from GitLab)

I don't think this is a bug in pyotb but a normal behaviour of OTB.
The snippet works if I re-run Execute() after WriteOutput() in write().
I guess this is not possible to re-use the app once it has been written, without re-executing it.
And it seems right, if the image is flushed to disk, it is not in memory anymore, so attributes like GetImageNbBands doesn't work anymore, if we don't execute once again.

I do believe what you want to do in code like test_pipeline isn't compatible with how otb works in memory. Generating a pipeline, writing the result to disk, then using it again in another pipeline as if it was still in memory... May be if the result has been written and is called again, we should point to the file on disk, instead of executing the app once more..?

By Vincent Delbar on 2023-01-06T14:14:43 (imported from GitLab)

~~No, it is possible (see my example in plain otbApplication above). I think its a bug in pyotb.~~

~~I have updated the issue description to highlight this.~~

By Rémi Cresson on 2023-01-06T16:19:04 (imported from GitLab)

Yes you're right, something must be wrong in write(), but the only diff I can see is that you're using ExecuteAndWriteOutput whereas we do Execute during app init then WriteOutput OR ExecuteAndWriteOutput...

By Vincent Delbar on 2023-01-06T14:26:14 (imported from GitLab)

AND write has to update parameters, so we edit params after Execute whereas you do it one shot in your snippet

By Vincent Delbar on 2023-01-06T14:27:04 (imported from GitLab)

Yes, that is strange: I am under the impression that Execute() is never called when write() only goes into the WriteOutput() branch...

By Rémi Cresson on 2023-01-06T14:29:31 (imported from GitLab)

My bad. Execute() is called during the __init__() of the app. I really don't understand for now

By Rémi Cresson on 2023-01-06T14:32:51 (imported from GitLab)

Well I tried to remove the try except at the end of write() by only self.app.ExecuteAndWriteOutput().

With this test_core is fixed ! But it breaks test_pipeline

By Vincent Delbar on 2023-01-06T14:33:04 (imported from GitLab)

Ho shit, here we go again

By Vincent Delbar on 2023-01-06T14:33:41 (imported from GitLab)

The code looks fine. Execute() and WriteOutput() are called in the right way. I don't see why it doesn't work

By Rémi Cresson on 2023-01-06T14:38:15 (imported from GitLab)

The code is fine. The following is failing in plain OTB:

app = otbApplication.Registry.CreateApplication("BandMath")
app.SetParameterStringList("il", [img_pth])
app.SetParameterString("exp", "im1b1")
app.Execute()
app.SetParameterString("out", "/tmp/toto.tif")
app.WriteOutput()
print(app.GetImageSize("out"))  # ERROR

I think that I understood the problem. Setting the "out" parameter triggers the UpdateParameters, so resets the state of the OTB application execution (m_ExecuteDone, etc)

By Rémi Cresson on 2023-01-06T14:49:00 (imported from GitLab)

This is supposed to fail since SetParameter calls UpdateParameters thus set ExecuteDone to false...

By Vincent Delbar on 2023-01-06T14:47:57 (imported from GitLab)

With ExecuteAndWriteOutput in write() in place of the try/except, test_pipeline fails with this good ol' error we know well

>       return _otbApplication.Application_ExecuteAndWriteOutput(self)
E       RuntimeError: Exception thrown in otbApplication Application_ExecuteAndWriteOutput: /build/insighttoolkit4-CEtaGF/insighttoolkit4-4.13.2-dfsg1/Modules/Core/Common/src/itkMultiThreader.cxx:399:
E       itk::ERROR: MultiThreader(0x2275cb10): Exception occurred during SingleMethodExecute
E       /usr/include/ITK-4.13/itkImageConstIterator.h:210:
E       itk::ERROR: Region ImageRegion (0x7ffc6a5fd410)
E         Dimension: 2
E         Index: [10, 0]
E         Size: [50, 5]
E        is outside of buffered region ImageRegion (0x218004e0)
E         Dimension: 2
E         Index: [0, 0]
E         Size: [0, 0]

By Vincent Delbar on 2023-01-06T14:51:18 (imported from GitLab)

OTB considers that any parameter modification implies that the application has not been executed. Even if it actually has. Sometimes, this behavior could make sense (e.g. changing some application parameters that change output image informations, like image size) but sometimes it doesn't (e.g. changing the output file name).

By Rémi Cresson on 2023-01-06T14:53:53 (imported from GitLab)

With ExecuteAndWriteOutput in write() in place of the try/except, test_pipeline fails with this good ol' error we know well

Yes and the application Execute() is called twice

By Rémi Cresson on 2023-01-06T14:54:59 (imported from GitLab)

Maybe we should manage some has_changed state in pyotb apps, where setting the output file names doesn't affect the state

By Rémi Cresson on 2023-01-06T14:58:15 (imported from GitLab)

Yep so quite bad. I see your comment above, but is there a way do distinct parameters that don't actually need to re-execute the app ?

By Vincent Delbar on 2023-01-06T14:58:31 (imported from GitLab)

I don't see what the has_changed would do if the problem is upstream in OTB code .?

By Vincent Delbar on 2023-01-06T14:59:10 (imported from GitLab)

is there a way do distinct parameters that don't actually need to re-execute the app ?

I think that the only parameters that do not need to re-execute the app are the output parameters

By Rémi Cresson on 2023-01-06T15:02:01 (imported from GitLab)

I don't see what the has_changed would do if the problem is upstream in OTB code .?

I don't know if this could be changed in OTB, regarding the architecture of the OTB application engine. I doubt that the OTB application engine is able to detect which parameters are modified during DoUpdateParameters(), which is implemented in the OTB applications. I think it would be a lot easier to do that on the python side.

By Rémi Cresson on 2023-01-06T15:06:21 (imported from GitLab)

Or... maybe it makes sense to consider in the OTB application engine, that when WriteOutput() is called, then m_ExecuteDone = True

By Rémi Cresson on 2023-01-06T15:17:04 (imported from GitLab)

It does !

By Vincent Delbar on 2023-01-06T15:18:58 (imported from GitLab)

I will propose a MR in OTB... you will tell me what you think ;) The shortest MR ever made _

By Rémi Cresson on 2023-01-06T15:22:30 (imported from GitLab)

It's working with has_changed and if we check this flag or call Execute in shape, to_array etc.
But this will be easier in my new branch because we need to init this flag, and right now otbObject doesn't have an __init__.
This is what I plan to change, by moving almost all the App code to OTBObject, to avoid having classes like Slicer that init an App object, while they also directly inherit from OTBObject.

By Vincent Delbar on 2023-01-06T15:23:51 (imported from GitLab)

But yes it would be better if we can avoid multiple execs...

By Vincent Delbar on 2023-01-06T15:22:46 (imported from GitLab)

https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb/-/merge_requests/941

By Rémi Cresson on 2023-01-06T15:41:52 (imported from GitLab)

Quite short indeed
Well I hope it can fix our problem, most solutions I tried either break test_pipeline or 8.1

By Vincent Delbar on 2023-01-06T16:01:06 (imported from GitLab)

It does

Writing /tmp/toto.tif...: 100% [**************************************************] (0s)
[251,304]

By Rémi Cresson on 2023-01-06T16:09:23 (imported from GitLab)

Merged today https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb/-/merge_requests/941

By Rémi Cresson on 2023-01-10T08:29:35 (imported from GitLab)

How long before the docker image gets an update ?

By Vincent Delbar on 2023-01-10T10:12:40 (imported from GitLab)

All right. I see this IsExecuteDone function, may be we need to check this everytime or call execute ( I mean in properties like shape, dtype, and in write...)

By Vincent Delbar on 2023-01-06T09:43:28 (imported from GitLab)

@remicress Looks like this IsExecuteDone function is not exposed to the python API, it would be great to add it !

By Vincent Delbar on 2023-01-06T09:44:29 (imported from GitLab)

changed the description

By Rémi Cresson on 2023-01-06T13:50:03 (imported from GitLab)

changed the description

By Rémi Cresson on 2023-01-06T13:50:59 (imported from GitLab)

changed the description

By Rémi Cresson on 2023-01-06T14:19:15 (imported from GitLab)

changed the description

By Rémi Cresson on 2023-01-06T14:20:19 (imported from GitLab)

changed the description

By Rémi Cresson on 2023-01-06T16:17:37 (imported from GitLab)

removed bug label

added bug waiting_for_otb_fix labels

removed bug label

https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb/-/merge_requests/932

By Rémi Cresson on 2023-01-11T18:51:30 (imported from GitLab)

closed

test_shape is broken since OTB 8.2

How to reproduce

Code

Output

Note

Designs

Child items 0

Activity