Add distance weighted particle interpolator #5815
Conversation
gassmoeller
force-pushed
the
distance_weighted_particle_average
branch
from
c7a14d4 to
3f0815b
Compare
gassmoeller
changed the title
gassmoeller
force-pushed
the
distance_weighted_particle_average
branch
2 times, most recently
from
d6eda59 to
792e91f
Compare
|
I cleaned up the PR and made sure I can run the particle benchmarks to show the method is converging. In terms of accuracy this interpolator has a similar convergence rate as 'cell average', but because the particle contribution is weighted by distance and particles of neighbor cells are considered as well, it should be less sensitive to particles crossing cell boundaries. Ready for review. |
gassmoeller
force-pushed
the
distance_weighted_particle_average
branch
from
792e91f to
b4d6fd7
Compare
| { | ||
| /** | ||
| * Return the interpolated properties of all particles of the given cell using bilinear least squares method. | ||
| * Currently, only the two dimensional model is supported. |
There was a problem hiding this comment.
You say it work in 3d, but here it is stated it only works in 2d. Can you update this?
| const typename parallel::distributed::Triangulation<dim>::active_cell_iterator &cell) const override; | ||
|
|
||
| // avoid -Woverloaded-virtual: | ||
| using Interface<dim>::properties_at_points; |
There was a problem hiding this comment.
Can you check if you still need this?
| { | ||
| namespace internal | ||
| { | ||
| double weight (const double distance, const double cell_width) |
There was a problem hiding this comment.
cell_width is not really the width of the cell but the width/radius of the hat function, maybe we should rename the argument?
| { | ||
| /** | ||
| * Return the interpolated properties of all particles of the given cell using bilinear least squares method. | ||
| * Currently, only the two dimensional model is supported. |
| vertex_to_cell_map[vertex_index].end()); | ||
| } | ||
|
|
||
| const double interpolation_support = 0.5 * cell->diameter(); |
There was a problem hiding this comment.
Do you make an assumption here that any particle that is less than this far away, will be found in the containing cell or its direct neighbors? Is that a safe assumption? This should probably be documented.
gassmoeller
force-pushed
the
distance_weighted_particle_average
branch
from
b4d6fd7 to
c353b18
Compare
|
I think I have addressed all comments, but let me run some more benchmarks with this before merging. |
gassmoeller
force-pushed
the
distance_weighted_particle_average
branch
from
c353b18 to
fd60a17
Compare
gassmoeller
force-pushed
the
distance_weighted_particle_average
branch
from
fd60a17 to
7694803
Compare
|
Ok, I am done benchmarking. I think this is ready. In the benchmark If anyone used Edit: Except that |
|
@gassmoeller - thanks for the work to add this feature! I did a bit of testing related to #4370 and ran into an issue when doing an initial adaptive refinement step on the first time step. The model runs without issue if The log.txt and a prm that be can quickly run on one processor are attached. Let me know how I can help with additional testing! |
|
@naliboff: Sorry about that I clearly didnt do enough testing for this interpolator, adaptive refinement was broken. I think I fixed it now and added a test. Could you give your model another try? I encountered another problem with your model that might be a bug in deal.II. If you crash still try changing the number of particles per direction to 6 (from 7). I will look into that more. |
|
@gassmoeller - Thanks for the fix and the model runs successfully now with both adaptive refinement and either 6 or 7 initial particles per direction. Is there any other additional testing I can do in the short term? I'll look more carefully at the effect of the weighted average scheme on convergence on the complex models in the coming weeks. |
There was a problem hiding this comment.
Looks good to me. Anything else you want to do here, @gassmoeller ?
|
This is ready from my side. Just a quick note that at present this interpolator is quite slow (about twice as slow than the linear least squares interpolator, which does a much more complicated operation). Presumably this is because it has to create the |
This is not ready for review.
@anne-glerum: This PR adds a new particle interpolator that interpolates between particles based on a distance weighted average (similar to what radial basis functions would do). The interpolator in its current form produces worse errors (though the same convergence rate) as the 'cell average' interpolator. However, it should be smoother and less affected by small particle movements across cell boundaries.
This PR is currently in a very rough state, I only open this to remind myself to finish it after the hackathon.