A potential issue with the `lower_star_img` function with negative numbers

[riccardoc95]

A potential issue with the `lower_star_img` function with negative numbers

[ctralie]

Okay, I made a new release with this problem fixed. Thank you again for pointing this out!

And yes, you are right, it is indeed the 8 nearest neighbors, not the 4. My apologies. Here's the second of the code where I did that:

ripser.py/ripser/ripser.py

Line 383 in 76da8ca

for di in [-1, 0, 1]:

[ctralie]

Ah, this is really interesting. What happens when you just print out -img? Perhaps keras is giving you back 8-bit unsigned ints and doing a negative is not what you expect?

…

On Thu, Jan 25, 2024 at 7:29 AM Riccardo Ceccaroni ***@***.***> wrote: Hello, I am using the lower_star_img function from your library, but I noticed that the output varies depending on the "type" of the input data. As an example, I'm taking the 29th image from the MNIST dataset in Keras. from ripser import lower_star_img from persim import plot_diagrams from tensorflow.keras import datasets (x_train, y_train), (x_test, y_test) = datasets.mnist.load_data() mnist = x_train img = mnist[29] 1. print(-lower_star_img(-img)): array([[ -1., -2.], [ -1., -5.], [ -1., -11.], [ -0., -inf]]) 1. print(-lower_star_img(-img.astype(np.float64))): array([[255., 254.], [255., 193.], [255., -inf]]) 1. print(255-lower_star_img(255-img.astype(np.float64))): array([[255., 254.], [255., 193.], [ 78., 0.], [255., -inf]]) I anticipate the correct response to be number 3. Is there a potential issue with the lower_star_img function when dealing with negative numbers? The introductory page <https://ripser.scikit-tda.org/en/latest/notebooks/Lower%20Star%20Image%20Filtrations.html> suggests it can handle negative inputs. Thank you very much. Looking forward to hearing from you soon! — Reply to this email directly, view it on GitHub <#153>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJWDZUKYG6QL47OP5U55ULYQJFY5AVCNFSM6AAAAABCKN6UW6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGEYDAMRZGE4TOMQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[riccardoc95]

Appreciate the prompt reply. Indeed, I apologize for the oversight; you're correct that in case 1, it's an overflow issue unrelated to Ripser. However, for cases 2 and 3, why isn't the component detected in case 2?

While I have your attention, could you please confirm whether the lower_star_img function utilizes simplicial complexes? Thank you!

[ctralie]

Darn, you're right, that is definitely a bug! I thought I handled this, and looking at the code it seems like I am handling it, but clearly there is a discrepancy for some examples (I found it for a different example in mnist). Anyway yes, we're using simplicial complexes with a center at each pixel and an edge between adjacent 4 neighbor pixels (left/right/up/down)

…

On Fri, Jan 26, 2024 at 1:37 AM Riccardo Ceccaroni ***@***.***> wrote: Appreciate the prompt reply. Indeed, I apologize for the oversight; you're correct that in case 1, it's an overflow issue unrelated to Ripser. However, for cases 2 and 3, why isn't the component detected in case 2? While I have your attention, could you please confirm whether the lower_star_img function utilizes simplicial complexes? Thank you! — Reply to this email directly, view it on GitHub <#153 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJWDZRFHGD3JN7QE3CGGWLYQNFKDAVCNFSM6AAAAABCKN6UW6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJRGU2TMOBSGM> . You are receiving this because you commented.Message ID: ***@***.***>

[ctralie]

I found the problem! It's a very subtle issue in ripser.cpp that only surfaces if an edge has exactly the value 0, which happens in mnist. I will write up a patch and push it tomorrow. Thank you for bringing this to my attention! On Sun, Feb 4, 2024 at 6:17 PM Christopher Tralie < ***@***.***> wrote:

…

Darn, you're right, that is definitely a bug! I thought I handled this, and looking at the code it seems like I am handling it, but clearly there is a discrepancy for some examples (I found it for a different example in mnist). Anyway yes, we're using simplicial complexes with a center at each pixel and an edge between adjacent 4 neighbor pixels (left/right/up/down) On Fri, Jan 26, 2024 at 1:37 AM Riccardo Ceccaroni < ***@***.***> wrote: > Appreciate the prompt reply. Indeed, I apologize for the oversight; > you're correct that in case 1, it's an overflow issue unrelated to Ripser. > However, for cases 2 and 3, why isn't the component detected in case 2? > > While I have your attention, could you please confirm whether the > lower_star_img function utilizes simplicial complexes? Thank you! > > — > Reply to this email directly, view it on GitHub > <#153 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAJWDZRFHGD3JN7QE3CGGWLYQNFKDAVCNFSM6AAAAABCKN6UW6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJRGU2TMOBSGM> > . > You are receiving this because you commented.Message ID: > ***@***.***> >

[riccardoc95]

Thanks!
I have a theoretical question lingering. You mentioned that each pixel is linked to the four adjacent pixels (left/right/up/down). However, on the lower_star_img webpage, it states "every vertex is connected to its 8 spatial neighbors". After conducting some tests, it appears to me that eight pixels are considered connected. Am I mistaken?

[ctralie]

Oh also, the release should sync with pypi sometime today, but for now, you can clone the most recent commit in master and type

pip install -e .

from the command line in that directory

[riccardoc95]

Thank you very much for the answers and also for the prompt bug fix!

[ctralie]

My pleasure, I'm really glad this worked out, and I'm grateful to you for bringing this up. I'm about to start using this code in a new research endeavor, so I'm glad to know that it's working better now