-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue when computing PH with maxdim == 0 #40
Conversation
Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
get_zero_apparent_cofacet was assuming that we were calling the function with maxdim > 0 inside of method compute_dim_0 Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
Now the test also computes with maxdim == 1 and compares barcodes in dimension 0 for both runs Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you for all the review Umbe, if all test passes, I will merge the PR. |
@MonkeyBreaker sure! I prefer squash and merge for a cleaner history (not sure if this is what you prefer too) |
I will then squash and merge, I prefer we stay consistent 😃 |
This PR fixes issue #39 when computing persistent homology with the maximal dimension set to 0.
The issue was that in function
compute_dim_0
, we also prepare the columns to reduce for dimensions 1. But, whenmaxdim=0
, we don't care about preparing those columns. Even worse was thatget_zero_apparent_cofacet(e, 1)
was assuming that we were always computing formaxdim>0
.This is now fixed by preparing the columns for dimension only if
maxdim > 0
.