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

Test DualView resize/realloc with WithoutInitializing for types without default constructor #7006

Merged

Conversation

masterleinad
Copy link
Contributor

This pull request adds another test for DualView for #6993 that fixed Kokkos::realloc/resize with Kokkos::WithouttInitializing such that it can be used with types that don't define a default constructor.

@masterleinad masterleinad force-pushed the test_no_default_constructor_dualview branch from cd43473 to cadab6c Compare May 10, 2024 19:47
@masterleinad masterleinad marked this pull request as ready for review May 10, 2024 20:15
Comment on lines +497 to +498
Impl::test_dualview_realloc<NoDefaultConstructor, TEST_EXECSPACE,
/* Initialize */ false>();
Copy link
Member

Choose a reason for hiding this comment

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

A little bit disturbing to have Impl:: on this line and not on the one above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

test_dualview_realloc calls Impl::test_dualview_realloc both with Initialiize=true and Initialize=false. We can only test with Initialize=false for NoDefualtConstructor, though.

Comment on lines +503 to +504
Impl::test_dualview_resize<NoDefaultConstructor, TEST_EXECSPACE,
/* Initialize */ false>();
Copy link
Member

Choose a reason for hiding this comment

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

A little bit disturbing to have Impl:: on this line and not on the one above.

Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

This doesn't work yet the failures in Jenkins are real.

@masterleinad masterleinad marked this pull request as draft May 16, 2024 13:30
@masterleinad masterleinad force-pushed the test_no_default_constructor_dualview branch from 9b0d839 to c07e218 Compare May 16, 2024 14:19
@masterleinad masterleinad force-pushed the test_no_default_constructor_dualview branch from c07e218 to 2b7b98a Compare May 16, 2024 14:57
@masterleinad masterleinad marked this pull request as ready for review May 16, 2024 16:13
@masterleinad masterleinad requested a review from crtrott May 16, 2024 17:36
@dalg24
Copy link
Member

dalg24 commented May 22, 2024

Retest this please

Copy link
Member

@crtrott crtrott left a comment

Choose a reason for hiding this comment

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

There is a SYCL segfault is that real?

@masterleinad
Copy link
Contributor Author

There is a SYCL segfault is that real?

No,

[ RUN ] sycl_graph.launch_six
3/57 Test #3: Kokkos_CoreUnitTest_SYCL1A .......................***Exception: SegFault 30.80 sec

was fixed in a different pull request and this one only modifies container tests anyway.

@masterleinad masterleinad requested a review from crtrott May 23, 2024 15:50
@crtrott crtrott merged commit 24b24d0 into kokkos:develop May 23, 2024
28 of 29 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.

None yet

4 participants