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
[WIP] New particle interpolation schemes #1333
[WIP] New particle interpolation schemes #1333
Conversation
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.
Very nice! Please address the minor issues, and resolve the conflict in changes.h.
<http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
#ifndef __aspect__particle_interpolator_bilinear_least_squares_h |
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 recently noticed that double underscores produce a warning in some modern compilers, and removed them from all other header files (they are reserved for compiler commands). Please replace them by single underscores (also in the following line, and in the other files).
namespace Interpolator | ||
{ | ||
/** | ||
* Return the cell-wise evaluated properties of the bilinear least squares function at the positions. |
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.
Please move this comment to the header file.
const std::vector<Point<dim> > &positions, | ||
const typename parallel::distributed::Triangulation<dim>::active_cell_iterator &cell) const | ||
{ | ||
AssertThrow(dim == 2, |
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.
Would it be easy to extend this to 3d, and would you be willing to extend it? If not that is totally fine, but then please add this limitation to the entry in changes.h, and to the description of the class, so that users are aware of 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.
Just saw that you mention it already in the description for the parameter file. Still, please also mention it in the class description of the header file, and the entry in changes.h.
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.
For now, I'll have only implemented the 2D case. The 3D case is in the works and will be added on soon.
Point<dim> approximated_cell_midpoint = positions[0]; | ||
if (positions.size() > 1) | ||
{ | ||
Tensor<1,dim> direction_to_center; |
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 there is an optimized version of this algorithm in interpolator/cell_average.cc
, please use that one if possible.
const typename parallel::distributed::Triangulation<dim>::active_cell_iterator &cell) const | ||
{ | ||
AssertThrow(dim == 2, | ||
ExcMessage("Currently, the particle interpolator 'biquadratic' is only supported for 2D models.")); |
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.
Same comment as above for the other class.
Point<dim> approximated_cell_midpoint = positions[0]; | ||
if (positions.size() > 1) | ||
{ | ||
Tensor<1,dim> direction_to_center; |
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.
Same as above.
There are several caveats to this PR.
|
b9eced7
to
b9d9c9c
Compare
/run-tests |
About point 1: If you are sitting on About point 2: I'm not sure how much anyone other than you guys can help with the debugging. Try to come up with as simple a problem as you can that shows the lack of convergence. Print output that shows things you think ought to be true (e.g., that the biquadratic interpolation's values should be within a certain range of values) with what actually is true in the implementation. Eventually you will come to a point where the implementation is doing something you know is not correct. Then figure out why this is so by going backward in the code from the point where something was already wrong. |
Very sorry for coming back so late to this PR. Could you rebase to a current master and resolve the conflict in chages.h?
I am not too much concerned going forward with this PR as long as 2. does not seem to interfere with practical applications. |
b9d9c9c
to
cc5d37b
Compare
@gassmoeller , as you probably know, in both the bilinear and biquadratic interpolation scheme, the least square method involves the inversion of a matrix. I use the deal.ii algorithm
To circumvent this, I have realized that I can run this model in Release mode but should I be concerned? Is this the algorithm of choice in terms of inverting a matrix? To reproduce this issue: Particles_Ra_1e5_B_1_k_3.prm.txt References: |
Yes, if something does not run in debug mode, you should always be very concerned. Ignoring errors in debug mode by just running in release mode is the same as just putting a piece of duct tape over the check engine light in your case because you don't like seeing it -- it doesn't make the problem go away. Is the problem reproducible? If so, can you output the offending matrix? If I recall the algorithm correctly, then a small or zero pivot indicates that the matrix is (nearly) singular. That's of course a problem, but it would be good to check that by looking at the entries of the matrix and verifying the issue. |
Well said! |
I believe that I may have found the cause or perhaps a second issue. Debugging this branch, I found the trail of a bug in my hastiness to keep up-to-date with #1384 . Interpolating a single particle property onto a single compositional field works as expected but interpolating two particle properties onto two compositional fields breaks down. I am using the aspect/tests/particle_interpolator_bilinear.prm added to this branch as my initial test case. After resolving this issue, if I then run into the issue of a pivot being close to zero during the inversion of the matrix, I will let you guys know ASAP. |
cc5d37b
to
50e1602
Compare
After double checking that the returned vector of vectors for the function For an active cell of level 0 and index 19, the constructed matrix A is
and taking the inverse,
once again resulted in
Using python3 module numpy, the inverse of A is computed to be
and A*A_inv is
In this problem, the dimensions of the 2D domain is 2e6 m by 6e6 m. After rescaling the domain such that it is 1 m by 3 m, for the active cell of level 0 and index of 19, the matrix A_rescale is then
and the inverse of A_rescale is
and A_rescale * A_rescale_inverse is
I am not sure what to make of these results. Any guidance would be much appreciated. |
If I understand you right, you are doing a least-squares fit onto a polynomial basis? Since you have a 3x3 matrix, can you say what basis you use? I'm slightly confused because you say you do a bilinear basis, but in 2d that would have 4 basis functions... |
I am attempting to fit the function The original reference is from the following paper where they have also excluded the x*y term: @Article{thielmann2014discretization, |
@bangerth, @hlokavarapu currently is doing a P1 least squares fitting, and return the values to composition field at supporting points with a Q1 basis, so it's essentially a P1 |
@bangerth @hlokavarapu , if I remember correctly, A_inverse.gauss_jordan() only explicitly gives the inverse matrix for matrix size less than 2x2, so for larger matrix size, it's solved implicitly by not forming the inverse matrix? Therefore, for a least squares fitting problem, the matrix may not be invertable or nearly singular, but it can still return a least square solution of the matrix system, which may give a warning and cause the trouble in the debug mode? |
OK, so it's a linear, not bilinear function space. I think that by itself should be fixed -- since we are working on quadrilaterals, it would be more appropriate to use the bilinear space (or the trilinear space in 3d). As far as the matrix is concerned: The monomial basis {1,x,y} leads to poorly conditioned problems if your variables are poorly scaled (as is the case here). I think a better approach would be to use the basis {1,x-x0,y-y0} where (x0,y0) are the coordinates of the cell center. But even that may be poorly suited; I would imagine that using the basis {1,(x-x0)/h,(y-y0)/h} is best, where h is the cell's diameter. All of this is a problem because in your case, x and y are variables that are very large, and so the factors |
@bangerth Thanks for your explanation. It seems well explained the current issue. :-) |
72bf553
to
3fa1409
Compare
…tic particle interpolation schemes
… the inverse of a singular matrix
cf5adf7
to
c7baa58
Compare
Added bilinear and biquadratic least squares method as particle property interpolation schemes with a simple overshoot and undershoot correction. Mainly, a minimum and maximum property value of all local particles within a cell is stored. If the interpolated value is larger than the local maximum or is smaller than the local minimum, then the interpolated value is set to the local maximum or local minimum property value, respectively.
Contributed by @yinghe616, @egpuckett, and myself.
Part 1 of #1319