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

Deprecate partition_master, validate_partition #4737

Merged
merged 3 commits into from
Feb 2, 2022
Merged

Deprecate partition_master, validate_partition #4737

merged 3 commits into from
Feb 2, 2022

Conversation

cz4rs
Copy link
Contributor

@cz4rs cz4rs commented Jan 31, 2022

Deprecate mentioned functions in preparation for their removal (see #4119).


This was briefly discussed at Kokkos Developer Meeting - please complain if you feel like voting is required.

@crtrott
Copy link
Member

crtrott commented Jan 31, 2022

Don't we need to protect the test or is that not getting compiled anymore?

Comment on lines +381 to +391
#ifdef KOKKOS_ENABLE_DEPRECATED_CODE_3
template <typename F>
static void partition_master(F const &, int requested_num_partitions = 0,
int = 0) {
KOKKOS_DEPRECATED static void partition_master(
F const &, int requested_num_partitions = 0, int = 0) {
if (requested_num_partitions > 1) {
Kokkos::abort(
"Kokkos::Experimental::HPX::partition_master: can't partition an "
"HPX instance\n");
}
}
#endif
Copy link
Member

Choose a reason for hiding this comment

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

@msimberg making sure you see this

Impl::OpenMPExec::verify_is_master("OpenMP::print_configuration");
#endif
Copy link
Member

@crtrott crtrott Feb 1, 2022

Choose a reason for hiding this comment

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

I think this needs to stay. Essentially the verify_is_master is there to prevent you from doing this:

#pragma omp parallel
{
   Kokkos;:parallel_for(N, functor);
}

Or do I misread that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you are right, I have been overzealous with this one. This should probably become deprecated after other changes from #4119 land.

@crtrott crtrott added the Blocks Promotion Overview issue for release-blocking bugs label Feb 1, 2022
@ajpowelsnl ajpowelsnl added this to In progress in Kokkos Release 3.6 via automation Feb 1, 2022
@ajpowelsnl ajpowelsnl linked an issue Feb 1, 2022 that may be closed by this pull request
4 tasks
@cz4rs cz4rs added this to In progress in Developer: Cezary Skrzyński via automation Feb 2, 2022
@cz4rs cz4rs marked this pull request as ready for review February 2, 2022 15:49
@cz4rs cz4rs requested a review from crtrott February 2, 2022 15:49
@cz4rs cz4rs changed the title Deprecate partition_master, validate_partition, verify_is_master Deprecate partition_master, validate_partition Feb 2, 2022
@dalg24
Copy link
Member

dalg24 commented Feb 2, 2022

Why did you decide to leave verify_is_master out?

@crtrott crtrott merged commit 07809a7 into kokkos:develop Feb 2, 2022
Kokkos Release 3.6 automation moved this from In progress to Done Feb 2, 2022
Developer: Cezary Skrzyński automation moved this from In progress to Done Feb 2, 2022
@ajpowelsnl
Copy link
Contributor

Morning @cz4rs -- Is this still a blocker for 3.6? I see something was merged a couple of weeks ago, but I'm not sure of its status now.

@cz4rs
Copy link
Contributor Author

cz4rs commented Feb 15, 2022

Morning @cz4rs -- Is this still a blocker for 3.6? I see something was merged a couple of weeks ago, but I'm not sure of its status now.

Hi Amy, I believe this is fine as is - the functions were marked deprecated as requested, other work (removal / refactoring) will be carried out in the next release.

@ajpowelsnl
Copy link
Contributor

Morning @cz4rs -- Is this still a blocker for 3.6? I see something was merged a couple of weeks ago, but I'm not sure of its status now.

Hi Amy, I believe this is fine as is - the functions were marked deprecated as requested, other work (removal / refactoring) will be carried out in the next release.

Thanks, Cezary! If this issue has been completed, then can we close it, with a view to opening new, related issue(s) for the next release?

@ajpowelsnl ajpowelsnl added Blocks Promotion Overview issue for release-blocking bugs InDevelop Enhancement, fix, etc. has been merged into the develop branch; and removed Blocks Promotion Overview issue for release-blocking bugs labels Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocks Promotion Overview issue for release-blocking bugs InDevelop Enhancement, fix, etc. has been merged into the develop branch;
Development

Successfully merging this pull request may close these issues.

None yet

5 participants