Skip to content

Scale MAPAXES to grid units in the opm-common grid reader - #14528

Open
magnesj wants to merge 2 commits into
devfrom
fix-mapaxes-unit-mismatch
Open

Scale MAPAXES to grid units in the opm-common grid reader#14528
magnesj wants to merge 2 commits into
devfrom
fix-mapaxes-unit-mismatch

Conversation

@magnesj

@magnesj magnesj commented Aug 14, 2026

Copy link
Copy Markdown
Member

Fixes #14527

opm-common scales MAPAXES to meter based on the MAPUNITS keyword, while COORD/ZCORN are left in the units given by GRIDUNIT. RigMainGrid::mapAxisTransform builds the transform from normalized axes plus an origin translation, so the origin must be in grid units to match the node coordinates. For a deck with GRIDUNIT FEET and MAPUNITS FEET the grid was displaced by origin * (1 - 0.3048).

The map axes are now scaled into grid units before use. The MAPUNITS string comparison mirrors EGrid.cpp exactly, since the intention is to detect what opm-common did rather than what the file says. When MAPUNITS is absent or unrecognized, opm-common leaves the values raw and no scaling is applied.

MAPUNITS GRIDUNIT opm-common gives scale result
(absent) FEET raw feet 1.0 feet
FEET FEET metres 1/0.3048 feet
METRES METRES metres 1.0 metres
FEET METRES metres 1.0 metres

The duplicated map axes block in RifReaderOpmCommon and RifReaderOpmCommonActive is moved into a shared applyMapAxes.

The guard is changed from get_mapaxes().size() == 6 to with_mapaxes(). The former is always true, as get_mapaxes() returns a std::array<double, 6>&, and EGrid::m_mapaxes is never zero initialized and only assigned when the MAPAXES keyword is present. A file without MAPAXES could therefore transform all nodes by uninitialized values.

opm-common scales MAPAXES to meter based on the MAPUNITS keyword, while COORD and ZCORN are left in the units given by GRIDUNIT. The map axis transform uses normalized axes plus an origin translation, so the origin must be in grid units to match the node coordinates. Scale the map axes accordingly, and use with_mapaxes() as the guard, as get_mapaxes() returns a fixed size array that is only assigned when the MAPAXES keyword is present. The duplicated map axes handling in the two readers is moved into a shared function.
@magnesj
magnesj requested a review from kriben August 14, 2026 13:38
RivSingleCellPartGenerator-Test.cpp and RivIjkIntersectionGeometryGenerator-Test.cpp both define buildBoxGrid in an anonymous namespace. The definitions do not collide as long as the two files end up in different unity build chunks, but adding a test file repartitions the chunks and the build then fails with a redefinition error.

@kriben kriben left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MAPAXES unit correction and shared helper look sound. I have one minor const-correctness suggestion.

//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
void RifReaderOpmCommon::applyMapAxes( Opm::EclIO::EGrid& opmGrid, RigMainGrid* mainGrid )

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applyMapAxes() only uses const accessors on opmGrid. Could this parameter be changed to const Opm::EclIO::EGrid&, including in the header? That would make the helper’s non-mutating contract explicit.

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

Successfully merging this pull request may close these issues.

MAPAXES unit mismatch in opm-common grid reader displaces the grid

2 participants