porespy icon indicating copy to clipboard operation
porespy copied to clipboard

`chords_apply` with `spacing=0` is incompatible with `chords_count`

Open bernarda78 opened this issue 2 years ago • 1 comments

The issue

I have found an issue with function chords_count on a chord filled image where spacing=0. The issue does not seems to appear when I use a spacing greater than 0. In my case, I'm interested by chords length in a binary image of 800^3 voxels and I end up with a 10^8 chord length using function chords_count. Similar to this previous issue (https://github.com/PMEAL/porespy/issues/131).

I suspect this commit (https://github.com/PMEAL/porespy/commit/68087927ab32f46eb0d1a21c74bdbbe937414a3f) introduced a regression for images that contained chords already labelled. Looking at the history, there seems to be a few back-and-forth commit, making the function unable to works with 0-spaced chord images, even though already labelled.

Possible fix

  • chords_count should check the image to know if it is labelled : a simple check like len(np.unique(im)) > 2 should be enough for most cases. There might a some corners cases that I have not though about.
  • another possibility would be for apply_chords to only return labelled image, as it appears in the code that labels are computed and removed at the end if not asked and remove the labeling in chords_count. This change will probably break backward compatibility.

If you want, I could make the change and a pull request once done.

bernarda78 avatar May 09 '22 15:05 bernarda78

Thanks for this report, and we appreciate the offer of a PR. I will take a look and get back to you.

jgostick avatar May 11 '22 02:05 jgostick

This function worked fine for me: Figure_1

jgostick avatar Mar 14 '23 19:03 jgostick