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

Interpolation point error #729

Merged
merged 9 commits into from
Dec 31, 2021
Merged

Interpolation point error #729

merged 9 commits into from
Dec 31, 2021

Conversation

oriolcg
Copy link
Member

@oriolcg oriolcg commented Dec 3, 2021

Fixing issue #727, closing #727 .

@oriolcg oriolcg added the bug Something isn't working label Dec 3, 2021
@oriolcg oriolcg requested a review from fverdugo December 3, 2021 08:35
@oriolcg oriolcg self-assigned this Dec 3, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2021

Codecov Report

Merging #729 (6838ed5) into master (bf394f5) will increase coverage by 0.32%.
The diff coverage is 98.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #729      +/-   ##
==========================================
+ Coverage   88.07%   88.40%   +0.32%     
==========================================
  Files         132      132              
  Lines       15038    15459     +421     
==========================================
+ Hits        13245    13666     +421     
  Misses       1793     1793              
Impacted Files Coverage Δ
src/CellData/CellFields.jl 88.19% <98.03%> (+0.19%) ⬆️
src/Arrays/CachedArrays.jl 73.17% <0.00%> (-1.19%) ⬇️
src/FESpaces/FESpacesWithConstantFixed.jl 75.58% <0.00%> (-0.67%) ⬇️
src/Geometry/Grids.jl 87.50% <0.00%> (-0.27%) ⬇️
src/Arrays/Tables.jl 94.97% <0.00%> (-0.27%) ⬇️
src/Arrays/Autodiff.jl 100.00% <0.00%> (ø)
src/Arrays/SubVectors.jl 100.00% <0.00%> (ø)
src/Arrays/AlgebraMaps.jl 100.00% <0.00%> (ø)
src/Helpers/GridapTypes.jl 100.00% <0.00%> (ø)
src/Fields/InverseFields.jl 100.00% <0.00%> (ø)
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf394f5...6838ed5. Read the comment docs.

@fverdugo
Copy link
Member

fverdugo commented Dec 7, 2021

@Balaje are you happy with this?

@oriolcg
Copy link
Member Author

oriolcg commented Dec 7, 2021

@fverdugo, I did an update trying to go in the direction that @Balaje suggested. I now use the searchmethod in Interpolable to define an optional argument with the number of nearest vertices (default = 1). This is much cleaner, this optional argument can be changed directly from the user driver (if needed) as follows:

sm=KDTreeSearch(num_nearest_vertices=10)
uₕ = interpolate_everywhere(Interpolable(uh_0;searchmethod=sm),U)

@Balaje
Copy link
Contributor

Balaje commented Dec 8, 2021

@oriolcg @fverdugo Yes, this is what I had in mind. It gives the user an option to provide a way to set the search level in case the algorithm misses points like this.

Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

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

@oriolcg Can you fabricate a simple example and add it that reproduces the bug that this PR wants to fix?

@oriolcg
Copy link
Member Author

oriolcg commented Dec 30, 2021

@fverdugo, I added the test in 6838ed5. The PR is ready

@fverdugo fverdugo merged commit 1dae811 into master Dec 31, 2021
@fverdugo fverdugo deleted the interpolation_point_error branch December 31, 2021 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants