-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added FastDEC #64
Added FastDEC #64
Conversation
These are some fundamental DEC operators that have been optimized for the DeltaSet ACSets.
These tests are adapted from the tests for DiscreteExteriorCalculus.
Shifted over all of the supporting code for the fast operators. This can probably be more organized but as it stands this allows the new operators to work, along with their tests and benchmarks.
Smashed mesh generation code into one single file, Meshes.jl, and brought over the Meshes test file from decapodes.
src/FastDEC.jl
Outdated
|
||
export dec_boundary, dec_differential, dec_dual_derivative, dec_hodge_star, dec_inv_hodge_star, dec_wedge_product | ||
|
||
# TODO: This relies on the assumption of a well ordering of the |
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.
I think some of these TODOs can be turned into XXXs, and maybe should be given in docstrings as well.
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.
I've added docstrings for the wedge products but I should also add them for the rest of the operators.
src/FastDEC.jl
Outdated
# points::Vector{Point3{Float64}} = sd[:point] | ||
# dual_points::Vector{Point3{Float64}} = sd[:dual_point] | ||
|
||
#TODO: Figure out how to type these since both Point2D and Point3D can be used |
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.
Can you pass the point type in as a type parameter of sd
?
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.
I've edited the hodges to have their sd
types be EmbeddedDeltaDualSets now and below are the results. The before is on the left, where we were only checking for AbstractDeltaDualComplexes and the right is the new with the stricter Embedded type. These benchmarks are ran on an Ico5 mesh.
There are clear improvements to making the types stricter and we should check to see if any other operators can benefit from this
Still should add docstrings for other fast operators. Also still need to look at improving Hodge 1 for 2D.
src/FastDEC.jl
Outdated
norm(v1v2) * (last(v1v2) == 0 ? 1.0 : sign(last(v1v2))) | ||
end | ||
|
||
function dec_hodge_star(::Type{Val{1}}, sd::AbstractDeltaDualComplex2D, ::GeometricHodge; float_type::DataType = Float64) |
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.
Since the Hodge star operator depends on the metric, it is inappropriate for this function to be called on any delta dual complex object, (such as OrientedDeltaDualComplex2D
), which does not have Point
information. (I think the theory technically does not require explicit points to require a metric, but our practical implementation depends on such.)
So, it is okay IMO for sd
to be explicitly ::EmbeddedDeltaDualComplex2D{_, _, point}
for this operator.
Also switched other operators to accept float_types from mesh itself, not as a keyword.
Some fundamental DEC operators that have been optimized for the DeltaSet ACSets are added in their own separate file. The reasons why these have been added into their own file is as follows:
The new DEC operators will probably not work or they may return bad results when the DualDeltaSet is somehow modified by beyond the subdivide_duals! method. However, there is no user restriction on modifying these ACSets so these operators are not "safe". This was the reason for the "fast_" prefix, implying that some assumptions are made to allow optimizations.
These new DEC operators sometimes return matrices and sometimes don't, specifically the Geometric Inverse Hodge for 1-Forms and the different Wedge Products. Since older implementations of more complicated operators, like Laplace de-Rham, only expect matrices this means that the old implementations cannot simply be replaced.
Separate from these concerns is that there is a desire to benchmark these operators but there are no large meshes stored with this package. Instead, these kinds of meshes are currently supported by the Decapodes package. So either mesh generation code must be upstreamed from Decapodes or the benchmarking will be done in that package, which is a strange division of responsibility.