imagej / imagej/imagej-ops

Divide and Multiply produce wrong results for Unsigned* types

Open
#541 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
94
Forks
44
PR merge metrics
No merged PRs in 30d

Description

As [discussed on gitter](https://gitter.im/imagej/imagej-ops?at=5a9683ca8f1c77ef3a664d18), the `math.divide` and `math.multiply` ops produce unexpected results when run on `Unsigned{Byte|Short}Type` data.

You can test with this Groovy script:
```groovy
#@ IOService io
#@ OpService ops

img = io.open("https://github.com/imagejan/sandbox/raw/wip/images/nucleus.png")
result = ops.run("math.divide", img, 32)
```

As @stelfrich [noticed](https://gitter.im/imagej/imagej-ops?at=5a9695af458cbde557daca0d):
> `Planar/ArrayImg` are backed by a `byte[]` and the op executes `plane[i] /= value`
for intensities > 127 that op will compute bogus results

The templates producing this code are here:

https://github.com/imagej/imagej-ops/blob/1375719d0ec1303601aef11d973ccbacda818580/src/main/templates/net/imagej/ops/math/ConstantToArrayImage.vm#L86-L91

https://github.com/imagej/imagej-ops/blob/1375719d0ec1303601aef11d973ccbacda818580/src/main/templates/net/imagej/ops/math/ConstantToPlanarImage.vm#L91-L96

In ImgLib2, the divide operation for `UnsignedByteType` is defined in their [`div()` method](https://github.com/imglib/imglib2/blob/af5f790c0fdd2b37bc8416dc1732ed57a6da145e/src/main/java/net/imglib2/type/numeric/integer/UnsignedByteType.java#L140-L143):

```java
public void div( final UnsignedByteType c )
{
set( get() / c.get() );
}
```

Maybe we should consider using the operators defined by the respective `Unsigned*` types, instead of doing calculations on the underlying `byte[]` arrays.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the ConstantToArrayImage.vm and ConstantToPlanarImage.vm template sections linked in the issue, then reproduce math.divide with the provided Groovy script. Compare the generated operations with UnsignedByteType.div(); done means divide and multiply produce correct results for UnsignedByteType and UnsignedShortType values, including intensities above 127.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
computer-vision
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.