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.

Sign up for GitHub

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

Jump to bottom

#264: Add serializers for Kokkos::pair and Kokkos::complex #267

Merged
merged 2 commits into from
Oct 25, 2022
Merged

#264: Add serializers for Kokkos::pair and Kokkos::complex #267

merged 2 commits into from
Oct 25, 2022

Conversation

thearusable
Copy link
Contributor

@thearusable thearusable commented Oct 24, 2022 •
edited
Loading

Fixes #264

@thearusable thearusable self-assigned this Oct 24, 2022
@PhilMiller
Copy link
Member

These should be simple enough that you can just do it in one PR, and say this PR fixes the issue

PhilMiller
PhilMiller requested changes Oct 24, 2022
View reviewed changes
Copy link
Member

@PhilMiller PhilMiller left a comment

Choose a reason for hiding this comment

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

The (de)serialization logic is way more complicated than I would have expected. Could you write a bit about why that's all necessary, and maybe add that motivation as comments in the code?

@thearusable thearusable changed the title #264: Add serializers for kokkos types (1/2) - Kokkos::Pair #264: Add serializers for Kokkos::pair and Kokkos::complex Oct 25, 2022
@thearusable thearusable marked this pull request as draft October 25, 2022 08:54
@thearusable
Copy link
Contributor Author

@PhilMiller Indeed! That could be accomplished in much simpler way. Thanks for catching that!

@thearusable thearusable marked this pull request as ready for review October 25, 2022 13:35
@PhilMiller PhilMiller merged commit 41e3e9c into develop Oct 25, 2022
@nmm0 nmm0 mentioned this pull request Oct 25, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhilMiller PhilMiller PhilMiller approved these changes

@cz4rs cz4rs Awaiting requested review from cz4rs

@lifflander lifflander Awaiting requested review from lifflander

@nlslatt nlslatt Awaiting requested review from nlslatt

Assignees

@thearusable thearusable

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add serializers for additional Kokkos types
2 participants
@thearusable @PhilMiller