More tests for evaluate #2
Conversation
Compressed assembly
| fhx = evaluate!(fhcache, fh, x) | ||
| @test fhx ≈ fx | ||
|
|
||
| for M ∈ ["Hypercubes", "Simplices"] |
There was a problem hiding this comment.
I would use symbols instead of strings:
for M \in [:hypercubes, :simplices]
...
M === :hypercubes ? ...There was a problem hiding this comment.
Fixed this. Waiting for @ericneiva to review the other changes.
src/CellData/CellFields.jl
Outdated
| ctype_to_polytope = map(get_polytope, ctype_to_reffe) | ||
| cell_map = get_cell_map(trian) | ||
| cache1 = kdtree, vertex_to_cells, cell_to_ctype, ctype_to_polytope, cell_map | ||
| cache1 = my_trian_data(trian) |
There was a problem hiding this comment.
I would rather give a more meaningful name to my_trian_data, such as point_to_cell_cache... what do you think?
src/CellData/CellFields.jl
Outdated
| @@ -368,6 +371,18 @@ function evaluate!(cache,f::CellField,point_to_x::AbstractVector{<:Point}) | |||
| collect(point_to_fx) # Collect into a plain array | |||
| end | |||
|
|
|||
| # New CellPoint implementation | |||
There was a problem hiding this comment.
This comment is doomed to be outdated, I would remove it. But you can (and should) inform that you have implemented this new procedure in the changelog file. Take a look at the structure of the file. What we do in a PR is to update the changelog with a summary of the changes being introduced (in the Unreleased section). I would add to the changelog that this PR adds (1) support to allow evaluation of FE functions at arbitrary points and (2) the new CellPoint implementation.
There was a problem hiding this comment.
I would also remove the comments within the CellPoint(xs::AbstractVector{<:Point}, trian::Triangulation, domain_style::PhysicalDomain) implementation. The code can be understood without them.
There was a problem hiding this comment.
Hi @ericneiva, I have updated the PR with these changes.
Ok, thanks a lot, @Balaje. From my side, this PR is good to go. |
|
Thanks so much @ericneiva Hi, @eschnett Please merge this PR if you're happy as well. Thank you. |
Hi @santiagobadia, @ericneiva, @oriolcg and @eschnett
I am submitting a pull request after adding some more tests for
evaluate. So far I have added test for:CellPoint. I might require some feedback for this. I had to exportmake_inverse_table,point_to_cell!anddistancefrom the implementation. I then use these functions for generating thecell_to_pointmap again, and pass the result toCellPointand then toevaluate.