DerThorsten / nifty

A nifty library for graph based image segmentation.
MIT License
41 stars 21 forks source link

Unexpected result from edgeWeightedWatershedsSegmentation() #125

Closed stuarteberg closed 5 years ago

stuarteberg commented 5 years ago

I use nifty.graph.edgeWeightedWatershedsSegmentation() in one of my libraries. So far, I've been using a rather old version (v0.24.0). I'm in the process of upgrading my compilers, so I tried upgrading my copy of nifty to v1.0.5. I'm seeing a strange result.

Here's a quick example, using the old version of nifty. (This just performs a watershed on the graph 0 - 1 - 2, using seeds 1 - 0 - 0.)

In [5]: seeds = np.array([1,0,0])

In [6]: edge_weights = np.array([0.5, 0.5])

In [7]: edges = np.array([[0,1], [1,2]])

In [8]: g = nifty.graph.UndirectedGraph(3)

In [9]: g.insertEdges(edges)

In [10]: nifty.graph.edgeWeightedWatershedsSegmentation(g, seeds, edge_weights)
Out[10]: array([1, 1, 1], dtype=uint64)

So far, so good. But if I repeat the above lines using nifty v1.0.5, I get this:

In [19]: nifty.graph.edgeWeightedWatershedsSegmentation(g, seeds, edge_weights)
Out[19]: array([0], dtype=uint64)

Since the graph has 3 nodes, shouldn't the result have shape (3,)? Or has the API changed? (As I said, I've been using a very out-of-date version.)

constantinpape commented 5 years ago

This is a bug in the pythonbindings that happened when switching everything to xtensor. For 1d pytensors, the constructor xt::pytensor<int, 1> x({len}) yields an array of shape (1,) with value len instead of shape (len,). Hence for 1d tensors one needs to use xt::pytensor<int, 1> x = xt::zeros<int>({len}). (Note that for d > 1 x({len_x, len_y}) works as expected).

Anyway I have fixed this for edgeWeightedWatersheds on my branch already but I wanted to check more thoroughly before merging. I will try to do this later and let you know.

constantinpape commented 5 years ago

@stuarteberg should be fixed on master now. Let me know if this works for you. Btw, I will try to put nifty on conda forge soon, see https://github.com/conda-forge/staged-recipes/pull/7763 and make a new release with these changes before that.

stuarteberg commented 5 years ago

Thanks, @constantinpape -- my tests are passing with the new version.