Skip to content

Concepts trial - #5434

Merged
WeiqunZhang merged 19 commits into
AMReX-Codes:developmentfrom
ThomasHowarth:concepts
May 28, 2026
Merged

Concepts trial#5434
WeiqunZhang merged 19 commits into
AMReX-Codes:developmentfrom
ThomasHowarth:concepts

Conversation

@ThomasHowarth

@ThomasHowarth ThomasHowarth commented May 6, 2026

Copy link
Copy Markdown
Contributor

I've had a go at adding in a couple of trial concepts into AMReX, specifically for the FillPatchUtil by adding in IsFabArrayType, IsBaseFabType and IsSpecificInterp concepts. The large number of changes mostly comes from the formatter (does anyone have a suggestion for that?).

A first step in #5375

@ThomasHowarth

Copy link
Copy Markdown
Contributor Author

I realise that most of these changes have also been suggested in #5372 - perhaps listing the concepts in AMReX_Concepts could make things more readable, however?

@WeiqunZhang

Copy link
Copy Markdown
Member

Adding AMReX_Concepts is not a bad idea, but the unnecessary white space changes are unacceptable.

The large number of changes mostly comes from the formatter (does anyone have a suggestion for that?).

Don't use it.

Comment thread Src/Base/AMReX_Concepts.H Outdated
* \brief Checks if a type is derived from amrex::BaseFab.
*/
template <typename T>
concept IsBaseFabType = amrex::IsBaseFab<T>::value;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As we know, naming is always the hardest thing. I don't think we should start with Is, because we might write code like below, which sounds awkward.

template <IsBaseFabType FAB> void f (FAB& fab);

Maybe we should use either BaseFabType or BaseFabLike.

Anyone who has an opinion, please feel free to chime in.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or FabType or FabLike?

@AlexanderSinn AlexanderSinn May 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with not having "Is" in the name. The standard library also doesn't have it.

template< class T >
concept floating_point = std::is_floating_point_v<T>;

Since BaseFAB is already taken, I think BaseFabType or FabType are fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've gone for BaseFabType/FabArrayType throughout

Comment thread Src/Base/AMReX_Concepts.H Outdated
* \brief Checks if a Interpolation is not generic MFInterpolater.
*/
template <typename Interp>
concept IsSpecificInterp = !std::is_same_v<Interp, amrex::MFInterpolater>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this concept is useful, because its meaning is very unclear. I think we should remove this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed. It is something that does seem to appear throughout - should it be left as is, or just a clearer concept?

Comment thread Src/AmrCore/AMReX_FillPatchUtil.H Outdated
@WeiqunZhang
WeiqunZhang merged commit 2e4fb93 into AMReX-Codes:development May 28, 2026
104 of 114 checks passed
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.

3 participants