Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert Box to BoxND #4016

Merged

Conversation

AlexanderSinn
Copy link
Member

@AlexanderSinn AlexanderSinn commented Jul 3, 2024

Summary

Similar to #3969 and #3988 but for Box.

Additional background

It should be checked that the changes to BoxIndexer do not affect the compiled GPU code.
In my testing, it gives the same performance as development. 

Example usage:

amrex::BoxND b1{amrex::IntVectND{1,2,3}, amrex::IntVectND{4,5,6}, amrex::IntVectND{1,0,1}};
// ((1,2,3) (4,5,6) (1,0,1))
auto b2 = amrex::BoxCat(b1, b1, b1);
// ((1,2,3,1,2,3,1,2,3) (4,5,6,4,5,6,4,5,6) (1,0,1,1,0,1,1,0,1))
auto [b3, b4, b5, b6, b7] = amrex::BoxSplit<1, 4, 2, 1, 1>(b2);
// ((1) (4) (1))((2,3,1,2) (5,6,4,5) (0,1,1,0))((3,1) (6,4) (1,1))((2) (5) (0))((3) (6) (1))
auto b8 = amrex::BoxResize<2>(b4);
// ((2,3) (5,6) (0,1))
auto b9 = amrex::BoxResize<5>(b8);
// ((2,3,0,0,0) (5,6,0,0,0) (0,1,0,0,0))

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

@AlexanderSinn
Copy link
Member Author

AlexanderSinn commented Jul 8, 2024

I added IntVectND::dim3(int fill_extra) to make it easier to implement BoxND functions such as end and length. It looks like pyamrex needs an overload_cast for dim3() now.

PR: AMReX-Codes/pyamrex#345

@AlexanderSinn AlexanderSinn changed the title [WIP] convert Box to BoxND Convert Box to BoxND Jul 28, 2024
@AlexanderSinn AlexanderSinn mentioned this pull request Jul 29, 2024
5 tasks
@ax3l
Copy link
Member

ax3l commented Jul 30, 2024

AMReX-Codes/pyamrex#345 merged - thanks!

*static_cast<Long>(length(1)),
*static_cast<Long>(length(2)))
: Long(0);
AMREX_IF_ON_HOST((checkOverflow();))
Copy link
Member

Choose a reason for hiding this comment

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

Could we put this inside ifdef AMREX_DEBUG? I don't think we need to check for overflow here for non-debug builds, given it's very unlikely overflow could happen here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also added defined(AMREX_USE_ASSERTION) as for me debug mode is often too slow to use.

amrex::min(b1.bigend [2], b2.bigend [2])),
typ);
#endif
BoxND<dim> minBox (BoxND<dim> const& b1, BoxND<dim> const& b2, IndexTypeND<dim> typ) noexcept
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this function. I don't think it's being used anywhere now. The name of this function is very misleading. So to avoid confusion, let's just remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

All minBox functions or just that one?

Copy link
Member

Choose a reason for hiding this comment

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

Just this one. This one has different behavior from others. It was added by me 5 years ago. I made a mistake in naming.

@WeiqunZhang
Copy link
Member

LGTM. I will do one more pass tomorrow.

@WeiqunZhang WeiqunZhang merged commit c09da99 into AMReX-Codes:development Aug 2, 2024
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants