Improving VoronoiDelaunayGrid by new FramedVoronoiGrid with fixed boundary and minimal distance between nodes

[sebastien-lenard]

The present implementation of VoronoiDelaunayGrid let the user construct the set of x-y coordinates of nodes that the grid receives as input. This has two drawbacks:

• perimeter (and boundary) nodes are not calculated properly. The issue was raised by Voroni "boundary" does not include all boundary nodes #885.
• the distances between two nodes can be very small. Particularly when the set of coordinates is pulled randomly. This occasionally triggers unstability in the models that includes diffusion or river incision (depending on the timestep of iterations).

To bypass these drawbacks, I propose to implement a new type of grid, the FramedVoronoiGrid, following a structure of classes and methods similar to the HexModelGrid. Four new classes are associated: the grid itself, FramedVoronoiGrid, a dual graph, DualFramedVoronoiGrid, which is parent of the grid, FramedVoronoiGraph, which is parent of DualFramedVoronoiGrid, and HorizontalRectVoronoiGraph, a static class implementing the methods necessary to instantiate a FramedVoronoiGraph.

The FramedVoronoiGrid starts by constructing a rectangular network of nodes. The boundary nodes are then fixed. This means that they are not determined by the ConvexHull as in the VoronoiDelaunayGrid. The core nodes are then moved around their position, in such a way that the distance between nodes don't exceed a certain threshold. While loosing the full potential of a fully random unstructured grid, the FramedVoronoiGrid is remains irregular and may fulfill some needs not currently fulfilled by the VoronoiDelaunayGrid or the HexModelGrid.

[sebastien-lenard]

Hi @mcflugen I merged the #1454 and let me know when I can view the changes on the edge node ids you implemented in your branch.

[sebastien-lenard]

@mcflugen it seems I missed your last comment on the pull request:

seeds used to generate the random x and y moves. This parameter is unused
when random_seed = True.

@sebastien-lenard Sorry, I still don't get why a user would want to have separate seeds for the x and y moves. The only reason I can think of is if they wanted the moves to be the same (i.e. not independent).

How about, instead, something like the following (seed is either an int or None),

rng = np.random.default_rng(seed=seed)
x_moves = rng.uniform(-max_move[0], max_move[0], shape)
y_moves = rng.uniform(-max_move[1], max_move[1], shape)

x_of_node[1:-1, 1:-1] += x_moves[1:-1, 1:-1]
y_of_node[1:-1, 1:-1] += y_moves[1:-1, 1:-1]

"
I'll do a new pull request to implement that.

[mcflugen]

I merged the #1454

@sebastien-lenard It wasn't quite ready to be merged yet. There was still the issue with the random number generator. No problem, though, I'll initiate a new pull request with my recommended changes for you to have a look at. (edit: our comments came through at the same time, how about you go ahead a create the pull request?).

I'll also create a new pull request with my proposed changes to the edge node ids.

[sebastien-lenard]

@mcflugen I created the pull request.

[mcflugen]

@mcflugen I created the pull request.

Thanks!

[sebastien-lenard]

@mcflugen if you want you can push your changes on this pull request, I think I achieved the modifications required to simplify to seed = one int.

My concern for your adaptation is whether I'll be able to know the node id of node Y for instance in the FramedVoronoiGrid below:
x-x-x-x
y-x-x-x
x-x-x-x
x-x-x-x

[mcflugen]

@sebastien-lenard Thanks. This is good to know.

Without any changes, I think the way you would do this is through the nodes_at_left_edge and nodes_at_right_edge arrays. I believe those arrays are ordered by row so, in your example, the id for node y would be grid.nodes_at_left_edge[2].

Another possibility. For a RasterModelGrid the nodes attribute is a 2D array of node ids that correspond to their position in the grid layout. For example,

>>> grid = RasterModelGrid((3, 4))
>>> grid.nodes
array([[ 0,  1,  2,  3],
       [ 4,  5,  6,  7],
       [ 8,  9, 10, 11]])

We could do something like that for FramedVoronoiGrid as well, I think. If so, the id of node y in your example would be grid.nodes[2, 0].

Do either of those ideas address your concern?

Rather than adding it to this pull request, I'll put this in a separate one to keep the two pull requests focused.

[sebastien-lenard]

Yes, @mcflugen the first idea addresses my concern. The nodes at left edge are ordered by node id.

For the second idea, the thing is that FramedVoronoiGrid might also be based on a hex or a radial grid (before random moves) in the future, so I guess it's not very flexible.