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

get_neighbors_of_at_offset returning multiple references to a single cell in refined grids #18

Open
alhom opened this issue Oct 12, 2020 · 5 comments

Comments

@alhom
Copy link

alhom commented Oct 12, 2020

The function get_neighbors_of_at_offset:

dccrg/dccrg.hpp

Line 3093 in b5ecae2

> get_neighbors_of_at_offset(

returns multiple references to a cell at the given offset if called in a refined grid: e.g. 8 copies of the same cell in the output in level 1 cells with a maxRefinement of 2.

@iljah
Copy link
Contributor

iljah commented Oct 12, 2020

Does this explain observed behavior: "Offset is in units of size of the given cell"?

@iljah
Copy link
Contributor

iljah commented Oct 12, 2020

If not, then program exhibiting this behavior would be best but knowing all input arguments, initial size and current structure of grid might suffice...

@markusbattarbee
Copy link

To clarify: the same neighbor cell (and it's indices) are returned multiple times. I think this occurs when both the cell and it's neighbor are at a lower refinement level than the grid includes. I'd wager that if it was called on level 0 cells on a maxRef 2 grid, we'd get 64 identical returns.

I think the problem arises from the fact that the function first calls indices_from_neighborhood(), which I think now returns all sets of indices contained within the queried cell or something. I'm not really sure why that is even called there, as then going through all neighbors, checking their indices, and seeing if they match the input values should already be enough.

@iljah
Copy link
Contributor

iljah commented Oct 12, 2020

Hmm doesn't look like this function is used anywhere within dccrg or by any tests so your mileage will probably vary :)

Looks like you're right, it's processing cells at index level instead of at level of cell given as argument and so returns many more results than it should. And it's implementation also looks suboptimal and doesn't use latest dccrg data structures, etc.

If you want exactly this functionality before I have time to fix this function (should be possible during this month) I suggest you do what I'll do and go through the neighbors_of iterator and skip cells that are not at desired offset. E.g. (after https://github.com/fmihpc/dccrg/blob/master/tests/advection/solve.hpp#L73)

for (const auto& cell: grid.local_cells(neighborhood_id)) {
  const int cell_length = grid.mapping.get_cell_length_in_indices(cell.id);
  for (const auto& neighbor: cell.neighbors_of) {
    if (neighbor.x > cell_length * desired_offset_x or neighbor.x < ...) { continue; }
    ...
    process(neighbor.id)
  }
}

@iljah
Copy link
Contributor

iljah commented Oct 12, 2020

Oh yeah neighbor.x,y,z should be documented in doxygen but I haven't really checked so looking at the source is probably best: https://github.com/fmihpc/dccrg/blob/master/dccrg.hpp#L7292

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

No branches or pull requests

3 participants