Multiple Breakages of Referential Transparency

[TomMD]

Immutability seems to have been thrown under a bus by a good number of operations. Take, for example, setROI. One would expect operation f to have the same result on the same input before and after setROI. Sadly, we are hiding immutable operations under the covers.

func = do
    img <- readImage somePath
    let bw1 = unsafeImageTo8Bit img
    when someBool (setROI (0,0) (200,200) img)
    saveImage "fullImage8bit.jpg" bw2

Now we should be able to use equational reasoning and say the image saved will indeed be the full image but black and white. Instead we get a cropped portion of the image thanks to the variable, img, being mutated. This certainly only gets worse when operations are hidden in other modules and not just inline with a when.

How should we go about solving this issue? Do you have plans on this front?

[TomMD]

I'm not intending to enumerate the extent of the breakage, but just because this one caught me off guard: countBlobs mutates the image. From OpenCV docs on findContours:

The function modifies the image while extracting the contours.

[TomMD]

OK, I've been thinking about this a little and I figure a good 8 hours of time could go to these fixes:

1) Add top-level signatures and comments. 2) Treat Image as an immutable object, allowing equational reasoning and avoiding confusions over lazy IO. Make a Unsafe or IO module (or set of modules) that includes:

mutableCopy :: Image c d -> IO (MutableImage c d)
unsafeImageToMutableImage :: Image c d -> MutableImage c d

and mutable IO operations will exist in this new layer of modules. At this point it might be worth asking if some sort of CV monad could benefit us in some way.

3) Re-type all necessary functions to be IO if they mutate things. 4) Fix most functions to make copies (ex: erode, dilate, threshold, countBlobs).

If these sound like changes you would support then let me know and I'll get some patches rolling.

[aleator]

Hmm. Thats indeed a huge mistake in the library.

Everything that mutates an image should be IO already and if it isn't, well then, thats a bug. The deeper mistake here is the interaction of otherwise pure functions, some IO and laziness. This needs to be fixed asap.

I've done the following:

* Bumped the version number of the head to 0.4
* Created a `MutableImage` type and auxiliary functions for it
* As a test, moved the obvious offender, `SetPixel`, to use this framework. (see #407b815)

Next, I'd suggest that, if you aim to contribute, that we each make tickets about modules that need to be sanitized and grab the ones we'd like to do. This is to avoid duplicate work.

However, just fixing everything to make copies is a good stop gap measure, but it will kill efficiency at some point. There is a small sliver of thought about this already in the ImageOp module, where the idea is to work with compositions of operations instead of images themselves and then run these compositions safely with copies.

(I'm however left wondering, if any of the recent haskell libraries such as pipes would have some basic idea we could also apply here)

[aleator]

Hmm.. I started mechanically working on this, but I think some thinking is in order first.

We'd like

• To use images in simple expressions such as img #- gaussian (w,h) img
• Protect against observing mutable images
• A way to optimize (hopefully automatically) expressions dealing with images so that a minimal number of copies is made.

I'm not sure how to get all three.

[aleator]

I made a preliminary attempt at purifying CV.Image (8003dc1). Does anyone have an extra pair of eyes to catch the missing/badly done functions?

The scheme currently is that only MutableImage can be mutated and this happens in IO. Also, observing anything about a mutable image should also happen in IO. Pure images can be observe purely and fed to pure functions.

When building pure images from using mutable operations a mutable may be unwrapped to pure and returned as is if the mutable version cannot be accessed afterwards and everything is explicitly evaluated so laziness doesn't result in funny effects.

I'm not super happy about exposing the Mutable constructor though..