Potential Problem in MULTREGTScanner

[bska]

Potential Problem in MULTREGTScanner

[blattms]

Actually, I don't see any relation of the global index of a cell with the extent of one region that would justify the previous version. To me the change looks like a bugfix.

We take the global index and compute i and j to determine whether the connection between the two cells is an NNC or not below.

[bska]

Actually, I don't see any relation of the global index of a cell with the extent of one region that would justify the previous version. To me the change looks like a bugfix.

Okay, then there's something I really don't understand. The usual way to compute the linearised Cartesian index of a zero-based IJK-tuple is

$$g = i + n_x\cdot(j + n_y\cdot k)$$

and the way to recover the IJK tuple from $g$ is then

$$i = g\, \mathrm{mod}\, n_x, \quad j = \lfloor g / n_x \rfloor\, \mathrm{mod}\, n_y, \quad k = \lfloor g / (n_x \cdot n_y) \rfloor$$

We take the global index and compute i and j to determine whether the connection between the two cells is an NNC or not below.

Sure, but the derivation of $j$, if it's actually supposed to be the J component of the cell's IJK index tuple, is suspicious to me since it uses nz instead of ny.

[blattms]

Sorry for my sloppy reading. Missed that aspect. That looks indeed strange/wrong. Also: why don't we also need to use k for the determination?

Maybe we should reuseGridDims::getijk here to prevent confusion?

[bska]

Missed [the $n_y$ vs. $n_z$] aspect. That looks indeed strange/wrong

Okay, cool. I was beginning to think I was losing my mind.

Maybe we should reuse GridDims::getijk here to prevent confusion?

Yes, I think that'd be a great idea. Unless you want to do the work, I can create a PR to that effect.

Also: why don't we also need to use k for the determination?

I don't know. It is, of course, possible that the original code did need to care about pinched-out layers, but nowadays the possibility that cells (I,J,K) and (I,J,K+3) would be connected across a pinch-out. I think that would classify as a non-neighbouring connection for the purpose of MULTREGT, but I'm not completely sure.

[blattms]

I don't know. It is, of course, possible that the original code did need to care about pinched-out layers, but nowadays the possibility that cells (I,J,K) and (I,J,K+3) would be connected across a pinch-out. I think that would classify as a non-neighbouring connection for the purpose of MULTREGT, but I'm not completely sure.

I think it does. All the documentatuion says so, all the code treats it like this (e.g. when writing out NNCs)

[blattms]

Concerning work: Yes please create the PR.

[bska]

Concerning work: Yes please create the PR.

Please see PR #3639 for my proposed change.

[blattms]

Closing this as the PR is merged.