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

Remove deprecated code in SortUtils #1171

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

aprokop
Copy link
Contributor

@aprokop aprokop commented Oct 4, 2024

  • Remove 2D and 3D permutation routines

@aprokop aprokop added the backward incompatible Modifications that may break users' code label Oct 4, 2024
Copy link
Contributor

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

You need to update the type requirements on apply[Inverse]Permutation()

PermuteHelper::CopyOp<OutputView, InputView>::copy(
output_view, permutation(i), input_view, i);
});
KOKKOS_LAMBDA(int i) { output_view(permutation(i)) = input_view(i); });
Copy link
Contributor

Choose a reason for hiding this comment

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

static assert that views are rank-1

@aprokop
Copy link
Contributor Author

aprokop commented Oct 4, 2024

You need to update the type requirements on apply[Inverse]Permutation()

What do you mean by type requirements? That the inputs are views?

@aprokop aprokop force-pushed the remove_deprecation branch from 6b8c35f to f962ec3 Compare October 4, 2024 20:35
@aprokop aprokop requested a review from dalg24 October 8, 2024 18:02
@dalg24
Copy link
Contributor

dalg24 commented Oct 9, 2024

You need to update the type requirements on apply[Inverse]Permutation()

What do you mean by type requirements? That the inputs are views?

that they are rank-1 views yes

Comment on lines +54 to +55
static_assert(InputView::rank == 1);
static_assert(OutputView::rank == 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static_assert(InputView::rank == 1);
static_assert(OutputView::rank == 1);
static_assert(InputView::rank() == 1);
static_assert(OutputView::rank() == 1);

Comment on lines +76 to +77
static_assert(InputView::rank == 1);
static_assert(OutputView::rank == 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static_assert(InputView::rank == 1);
static_assert(OutputView::rank == 1);
static_assert(InputView::rank() == 1);
static_assert(OutputView::rank() == 1);

@aprokop
Copy link
Contributor Author

aprokop commented Oct 9, 2024

Thanks for the rank() suggestions. As there are multiple places in our code, I'll do them in a separate PR.

@aprokop aprokop merged commit 62caef6 into arborx:master Oct 9, 2024
2 checks passed
@aprokop aprokop deleted the remove_deprecation branch October 9, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backward incompatible Modifications that may break users' code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants