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

Add cuda-compatible ippl::Tuple with test #250

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

manuel5975p
Copy link
Contributor

Add __device__-compliant ippl::Tuple type, inspired by std::tuple

Currently only in one file Types/Tuple.h, since most functions are trivial one-liners.

Some examples:

{
    ippl::Tuple<int, std::unique_ptr<int>> tuple1(17, std::make_unique<int>(5));
    ippl::Tuple<int, std::unique_ptr<int>> tuple2{5, nullptr};
    //tuple2 = tuple1 is ill-formed
    tuple2 = std::move(tuple1); //Moves correctly
    
    ippl::Tuple<int, double> values1{5, 10.0};
    ippl::Tuple<int, double> values2(0.5f, 1.0f); //Both work, 0.5f is casted to 0 (type int).

    std::cout << ippl::get<0>(values1 + values2); // 5
    std::cout << ippl::get<1>(values1 - values2); // 9.0
}

Arithmetic operators are only enable for tuples in which all types satisfy is_arithmetic

Tests work for Serial, OpenMP and Cuda, including a parallel_for and a parallel_reduce loop.

A usecase for this is Boundary Conditions for FDTD (and also general BCs) since the axis along which the wave vanishes should be a template parameter.

Copy link
Contributor

@Arc676 Arc676 left a comment

Choose a reason for hiding this comment

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

Could use some additional empty lines to space out some things but otherwise seems good.

#include <memory>
#include <Ippl.h>
//#include <Kokkos_Core.hpp>
#define alwaysAssert(X) do{if(!(X)){std::cerr<<"Assertion " << #X << " failed: " << __FILE__ << ": " << __LINE__ << '\n'; exit(-1);}}while(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would split this macro over several lines for readability.

However, this program also feels more like a unit test. Instead of building your own assert, you could just use the ones already in GTest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, that was a bit of a hack, also I just went off tests/vector

Copy link
Contributor

Choose a reason for hiding this comment

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

@aaadelmann What do you think? I would argue that the nature of this program makes it more suited to be included as a unit test. I had previously argued in favor of trimming most of the test/ programs anyway in favor of converting them to more helpful, illustrative example programs and restricting actual internal testing to either the unit tests or programs that are not tracked with the repository, or just minimal testbeds.

Copy link
Member

Choose a reason for hiding this comment

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

@manuel5975p please open an issue for the unit test! @Arc676 please approve

src/Types/Tuple.h Outdated Show resolved Hide resolved
src/Types/Tuple.h Outdated Show resolved Hide resolved
@aaadelmann aaadelmann merged commit dc7d21e into IPPL-framework:dev-3.2.0 Mar 14, 2024
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.

3 participants