-
Notifications
You must be signed in to change notification settings - Fork 166
Las Vegas algorithm for exterior algebraic shifting #5587
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
base: master
Are you sure you want to change the base?
Conversation
…g-shifting-post-issac
| @doc raw""" | ||
| complex_faces(K::SimplicialComplex) | ||
| Returns a Set{Set{Int}} containing all simplices of K. | ||
| complex_faces(K::SimplicialComplex, dim::Int) | ||
| Returns a Set{Set{Int}} containing all d-simplices of K. | ||
| """ | ||
| complex_faces(K :: SimplicialComplex) = union(complex_faces_by_dimension(K)...) :: Set{Set{Int}} | ||
| complex_faces(K :: SimplicialComplex, dim::Int) = complex_faces_by_dimension(K, mindim=dim)[dim+1] :: Set{Set{Int}} |
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.
The docstrings in this file are not properly formatted.
This function should probably be renamed to just faces (we have the same for polyhedra, without the variant without a dimension).
I think complex_faces_by_dimension should be removed, faces(K,dim) can be implemented directly without first allocating a vector for all dimensions. And faces(K) could use elements(p::PartiallyOrderedSet) instead of splatting+union.
| return uniform_hypergraph(faces, n, only(Set(length.(faces)))) | ||
| end | ||
| const CollectionTypes = Union{Set{Set{Int}}, Vector{Vector{Int}}, Vector{Combination{Int}}} | ||
| uniform_hypergraph(faces::CollectionTypes, n::Int, k::Int) = UniformHypergraph(n, k, sort(collect(Set(sort.(collect.(Set.(faces))))))) |
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.
Even though this was not really changed in this PR that sort(... looks weird and inefficient (converting to a (unordered) set first and then sorting it).
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.
see latest commits
| end | ||
|
|
||
| function load_object(s::DeserializerState, K::Type{UniformHypergraph}) | ||
| return uniform_hypergraph(load_object(s, Vector{Vector{Int}})) |
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.
We could assume that the data is properly sorted and avoid the re-sort on load?
(But if we change the construction to avoid at least some of the sorting this could be ok?)
| end | ||
| for k=i+1:nrows(M) | ||
| M[k, :] = M[i, j]*M[k, :] - M[k, j] * M[i, :] | ||
| M[k, :] = M[i, j]*M[k:k, :] - M[k, j] * M[i:i, :] |
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.
Why is this changed here? If it was wrong then a test would be good.
| Returns a Vector{Set{Set{Int}}} fs such that fs[d+1] contains all d-simplices of K. | ||
| """ | ||
| function complex_faces_by_dimension(K :: SimplicialComplex; mindim::Int=0) |
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.
This doesn't use the mindim argument (but I suggested to remove this function anyway).
And it currently starts from rank 1 so it would omit the bottom element (i.e. the empty face of dimension -1).
Co-authored-by: Benjamin Lorenz <[email protected]>
…g-shifting-post-issac
las vegas algorithm for computing the exterior algebraic shifting.
Includes improvements that takes advantage of the lower dimensional faces and eliminates higher co faces when the result is a shifted complex.