PlaneSampler skipping normal unless offsets is specified

[rthedin]

When using the PlaneSampler, the normal (the same as axis3) does not get populated unless offsets is specified:

amr-wind/amr-wind/utilities/sampling/PlaneSampler.cpp

Lines 28 to 29 in 3be7419

	if (noffsets > 0) {
	pp.getarr("normal", m_normal);

Without offsets, axis3 is 0, 0, 0, regardless of the value of normal in the input file.

[rybchuk]

I thought this was intentional. normal comes up when you have offsets to specify the direction of the offset, but you don't need that for the single plane.

[marchdf]

I think (?) that this is a classic case of a thing being used somewhere else where it shouldn't be/bad naming/bad reuse. normal as @rybchuk says is really for specifying the direction along which to add offset planes. It is not technically the "normal" of the plane, that would be given by the cross product of axis1 and axis2.

axis3 in the netcdf output uses assuming it was set. But it's not the normal of the plane. It indicates the direction along which the offset planes were done.

This is confusing. But I am not sure what the correct fix is because this is all user facing options. Rename "normal" in the input file and break a bunch of user input files (maybe with a grace period with warnings supplied)? Rename axis3 in the netcdf output to something more descriptive? axis3 -> offset_extrusion_direction ? And then break some postprocessing scripts?

Or did I just get all this wrong and something else is going on...

[marchdf]

I hesitate to make axis3 = axis1 x axis2 if normal is not set because that's kinda changing the definition of "axis3" depending on the offset inputs...

[marchdf]

Maybe this picture helps the discussion? Or shows where I am wrong.

[rthedin]

I figured there was a chance this was all intentional. You got my point correctly, Marc. Your picture is consistent with what is currently happening. I got used to set normal regardless of offsets and not having normal broke my post-processing scripts for someone else. I personally think a different name like the one you suggested would be best, but I see the problem of renaming and breaking people's processing scripts. I defer the choice of renaming or not renaming to you guys. Thanks for the clarification!

[lawrenceccheung]

Yes, I believe the sample plane inputs were intentionally set up this way. We set it up so it used the same vectors that are in Nalu-Wind's plane_specifications, and the intent was to allow the situation that @marchdf sketched. Unfortunately normal is a poor choice of words, Nalu-Wind uses the offset_vector terminology:

      - name: sliceData/p_hub
        corner_coordinates: [0, 0, 90]
        edge1_vector: [5120, 0, 0]
        edge2_vector: [0, 5120, 0]
        edge1_numPoints: 513
        edge2_numPoints: 513
        offset_vector:   [0, 0, 1]
        offset_spacings: [0, 20, 40]

[marchdf]

I don't like messing with the users but I am worried this is going to come up again. See #1137

[marchdf]

I am closing this because the PR won't be merged until the next release but will be addressed. Please reopen if not.