Skip to content
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

Simplex tree expansion_with_blockers python interface #389

Merged

Conversation

VincentRouvreau
Copy link
Contributor

@VincentRouvreau VincentRouvreau commented Aug 28, 2020

This is a prototype based on https://github.com/cython/cython/tree/master/Demos/callback

from gudhi import SimplexTree
st=SimplexTree()

def blocker(simplex):
    try:
        # Block all simplices that countains vertex 6
        simplex.index(6)
        print(simplex, ' is blocked')
        return True
    except ValueError:
        print(simplex, ' is accepted')
        print(simplex, " = ", st.filtration(simplex))
        st.assign_filtration(simplex, 12.)
        return False

st.insert([0,1],0)
st.insert([0,2],1)
st.insert([0,3],2)
st.insert([1,2],3)
st.insert([1,3],4)
st.insert([2,3],5)
st.insert([2,4],6)
st.insert([3,6],7)
st.insert([4,5],8)
st.insert([4,6],9)
st.insert([5,6],10)
st.insert([6],10)

st.expansion_with_blocker(2, blocker)
print(list(st.get_filtration()))
[6, 5, 4]  is blocked
[3, 2, 1]  is accepted
[3, 2, 1]  =  5.0
[3, 2, 0]  is accepted
[3, 2, 0]  =  5.0
[2, 1, 0]  is accepted
[2, 1, 0]  =  3.0
[3, 1, 0]  is accepted
[3, 1, 0]  =  4.0

[([0], 0.0), ([1], 0.0), ([0, 1], 0.0), ([2], 1.0), ([0, 2], 1.0), ([3], 2.0), ([0, 3], 2.0), ([1, 2], 3.0), ([1, 3], 4.0), ([2, 3], 5.0), ([4], 6.0), ([2, 4], 6.0), ([6], 7.0), ([3, 6], 7.0), ([5], 8.0), ([4, 5], 8.0), ([4, 6], 9.0), ([5, 6], 10.0), ([0, 1, 2], 12.0), ([0, 1, 3], 12.0), ([0, 2, 3], 12.0), ([1, 2, 3], 12.0)]

@VincentRouvreau VincentRouvreau linked an issue Sep 1, 2020 that may be closed by this pull request
@VincentRouvreau VincentRouvreau changed the title A prototype to fix #364 Simplex tree expansion_with_blockers python interface Sep 1, 2020
@VincentRouvreau VincentRouvreau marked this pull request as ready for review September 1, 2020 13:14
@@ -66,6 +66,9 @@ cdef extern from "Simplex_tree_interface.h" namespace "Gudhi":
vector[Simplex_tree_simplex_handle].const_iterator get_filtration_iterator_end() nogil
Simplex_tree_skeleton_iterator get_skeleton_iterator_begin(int dimension) nogil
Simplex_tree_skeleton_iterator get_skeleton_iterator_end(int dimension) nogil
# Expansion with blockers
ctypedef bool (*blocker_func)(vector[int], void *user_data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use blocker_func both as a type here and as the name of a variable in another file. Maybe add _t for the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed on f4f5992

otherwise it is kept. The algorithm then proceeds with the next candidate.

Note that you cannot update the filtration value of the simplex during the evaluation of `blocker_func`, as it
would segfault.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C++ version allows that, it would be good to understand what is going wrong here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this paragraph is now wrong?
The C++ doc has a warning about phantom simplices that should also be in the python doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The note was removed on 924302f
Add a warning about phantom vertices in f6a4524

:param max_dim: Expansion maximal dimension value.
:type max_dim: int
:param blocker_func: Blocker oracle.
:type blocker_func: Its concept is `Boolean blocker_func(list of int)`
Copy link
Member

@mglisse mglisse Sep 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:type blocker_func: Callable[[List[int]], bool]
according to https://docs.python.org/3/library/typing.html#callable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done on f4f5992

@VincentRouvreau

This comment was marked as resolved.

@mglisse
Copy link
Member

mglisse commented Mar 18, 2022

Is it because simplex->second.assign_children(new_sib); happens too late, would moving it earlier (and cancelling it in the case where we erase everything) help?
(looking at this code, the quadratic erase loop doesn't look so good, but that's just a performance question)

@VincentRouvreau
Copy link
Contributor Author

Is it because simplex->second.assign_children(new_sib); happens too late, would moving it earlier (and cancelling it in the case where we erase everything) help? (looking at this code, the quadratic erase loop doesn't look so good, but that's just a performance question)

Nice ! Thanks ! It has been done on d114eaf

@VincentRouvreau VincentRouvreau marked this pull request as ready for review March 28, 2022 07:07
@VincentRouvreau VincentRouvreau merged commit b066b43 into GUDHI:master Mar 31, 2022
@VincentRouvreau VincentRouvreau deleted the expansion_with_blockers_python_itf branch March 31, 2022 14:50
@VincentRouvreau VincentRouvreau added the 3.6.0 GUDHI version 3.6.0 label Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.6.0 GUDHI version 3.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SimplexTree] interface expansion_with_blockers
2 participants