Optimizing SpecificInd::zp(), SpecificInd::zm()?

[johnomotani]

There's a check on the size of the argument to zp(int dz) and zm(int dz)

BOUT-dev/include/bout/region.hxx

Lines 288 to 292 in 6755324

	const inline SpecificInd zp(int dz = 1) const {
	ASSERT3(dz >= 0);
	dz = dz <= nz ? dz : dz % nz; //Fix in case dz > nz, if not force it to be in range
	return {(ind + dz) % nz < dz ? ind - nz + dz : ind + dz, ny, nz};
	}

It's not used very much, but the commonest use is with no argument. Instead of having the default argument of dz = 1, would it be worth having a special no-argument case that does not need the check on dz (as if nz == 1 the result is always 0 anyway)? And possibly another for zpp() and zmm() where we could have something like ASSERT3(nz >= 2); instead of always checking dz?

[dschwoerer]

Providing optimised version seems like a good idea as % is rather slow.

Are you really meaning nz? Why would we assert nz>1?

[d7919]

I'd be supportive of this in general. The branching in these routines mean that loops calling zp/zm won't be vectorised at the moment, one of the goals of guard cell based periodicity is to avoid some of this logic and enable vectorisation -- the zp/zm routines would then look just like the xp etc. versions.

I'm not sure how much this first line dealing with setting dz will impact on performance though. I don't think there should be much cost in the typical case where dz <= nz as we end up just saying dz = dz and won't evaluate the dz % nz branch. We can't remove the conditional in the return statement so I think removing the earlier check will not be sufficient to allow vectorisation. I think the main place we might hit the extra modulus is in the use of zpp/zmm with Ind2D where nz == 1.

[johnomotani]

I was thinking we could check nz for zpp in case there is some special case if nz==1 or nz==2. I think those cases both work OK though, because for nz==1 the result is always 0 and for nz==2 the result will be equal to ind%2 without changing dz from 2 to 0.