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

Bug on find_active_cell_around_point for p::d::t. #10175

Closed
luca-heltai opened this issue May 13, 2020 · 8 comments
Closed

Bug on find_active_cell_around_point for p::d::t. #10175

luca-heltai opened this issue May 13, 2020 · 8 comments

Comments

@luca-heltai
Copy link
Member

luca-heltai commented May 13, 2020

PR #10174 reminded us that there is a problem in the fast algorithm, so we had to resort to the slow function

          return find_active_cell_around_point(mapping,
                                               mesh,
                                               p,
                                               marked_vertices);

in case the initial search did not succeed. This should be investigated and fixed.

@kronbichler
Copy link
Member

Adjusting to the 9.3 milestone now that we have the temporary solution.

@luca-heltai
Copy link
Member Author

So, this has been around for a long time: #7576. I think it is time we think of a permanent solution... (see also #10459, and #10506)

@nfehn
Copy link
Contributor

nfehn commented Jun 15, 2020

I currently encounter problems with the fast GridTools::find_active_cell_around_point() function. My question is therefore: To which extent can one expect that the slow and fast versions give the same result? If the algorithm returns the first cell that is found, it might be that the slow and fast functions return different cells, is that correct? However, I call GridTools::find_all_active_cells_around_point() afterwards to find all cells, so I would expect that in this case the slow and fast versions (with a subsequent call of find_all_...()) must give the same result?

@nfehn
Copy link
Contributor

nfehn commented Jun 15, 2020

So, this has been around for a long time: #7576. I think it is time we think of a permanent solution... (see also #10459, and #10506)

@luca-heltai what is the relation to #7930, which you did not mention above?

@nfehn
Copy link
Contributor

nfehn commented Jun 17, 2020

The problem mentioned above will be solved by PR #10540

@bangerth
Copy link
Member

@nfehn You mention that this should have been fixed by a patch a couple of years ago. Can you confirm that this is so?

@tamiko tamiko modified the milestones: Release 9.4, Release 9.5 Jun 14, 2022
@kronbichler
Copy link
Member

I think we made several improvements during the last release. However, in exadg/exadg#438 we observed another problematic case. I am not sure if we can fix this entirely for the upcoming 9.5 release, but I think we also need to look over the interfaces, because there are too many functions doing similar things in GridTools. Let us keep this on the milestone for now.

@kronbichler
Copy link
Member

I'm closing this issue, given the progress in #15233. We might have additional bugs or optimizations, but we should make those based on up-to-date test cases and/or code restructuring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants