Skip to content

Reduce duplication in Coordinates constructors#3372

Open
ZedThree wants to merge 10 commits into
nextfrom
coordinates-split-ctor
Open

Reduce duplication in Coordinates constructors#3372
ZedThree wants to merge 10 commits into
nextfrom
coordinates-split-ctor

Conversation

@ZedThree

Copy link
Copy Markdown
Member

We had a lot of duplicated complicated code in the Coordinates initialisation, this attempts to reduce it. This should make the refactor in #2873 smaller and easier to review as well

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/mesh/coordinates.cxx
readFromMesh(options, "");
}

Coordinates::Coordinates(Mesh* mesh, Options* options, const CELL_LOC loc,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: non_uniform [cppcoreguidelines-pro-type-member-init]

Coordinates::Coordinates(Mesh* mesh, Options* options, const CELL_LOC loc,
^

Comment thread src/mesh/coordinates.cxx
Comment thread src/mesh/coordinates.cxx Outdated
Comment thread src/mesh/coordinates.cxx Outdated
Comment thread src/mesh/coordinates.cxx Outdated
Comment thread src/mesh/coordinates.cxx Outdated
Comment thread src/mesh/coordinates.cxx Outdated
@ZedThree ZedThree force-pushed the coordinates-split-ctor branch 3 times, most recently from 7aff4fe to 4a08cb3 Compare May 14, 2026 15:54

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/mesh/data/gridfromoptions.cxx
@ZedThree ZedThree force-pushed the coordinates-split-ctor branch from abca9c2 to a976e78 Compare June 15, 2026 14:27
@ZedThree ZedThree force-pushed the coordinates-split-ctor branch from a976e78 to a3d0999 Compare June 15, 2026 16:36

@dschwoerer dschwoerer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer to revert the last two commits, as it requires as reading the fields twice, so it is imho not that much cleaner then the old design.

Comment thread src/mesh/coordinates.cxx
Field3D interpolateAndExtrapolate(const Field3D& f_, CELL_LOC location,
bool extrapolate_x, bool extrapolate_y,
bool no_extra_interpolate, ParallelTransform* pt_) {
/// Interpolate a Field2D to a new CELL_LOC with interp_to.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Interpolate a Field2D to a new CELL_LOC with interp_to.
/// Interpolate a Field to a new CELL_LOC with interp_to.

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.

2 participants