Skip to content

Improve backwards compatibility#28

Open
bmhowe23 wants to merge 1 commit intobettinaheim:spin_opsfrom
bmhowe23:spin_ops-bmh2
Open

Improve backwards compatibility#28
bmhowe23 wants to merge 1 commit intobettinaheim:spin_opsfrom
bmhowe23:spin_ops-bmh2

Conversation

@bmhowe23
Copy link

@bmhowe23 bmhowe23 commented Mar 10, 2025

I think this small set of changes makes a tremendous difference in the backwards compatibility of the PR.

From a build perspective, this cuts the number of lines of codes that CUDA-QX would need to change probably by >90%.

I am not convinced in the correctness of the default constructor, though, so please let me know if something looks glaringly wrong (especially regarding the default constructor).

Signed-off-by: Ben Howe <bhowe@nvidia.com>
sum_op<spin_handler> x(std::size_t target);
sum_op<spin_handler> y(std::size_t target);
sum_op<spin_handler> z(std::size_t target);
product_op<spin_handler> i(std::size_t target);
Copy link
Author

Choose a reason for hiding this comment

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

These changes allow you to create spin_op_term's from i,x,y,z.

Copy link
Owner

@bettinaheim bettinaheim Mar 10, 2025

Choose a reason for hiding this comment

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

This is what I had originally, but if you want backward compatibility, it's a sum here. The product version that should be used is spin_op::x

Copy link
Author

@bmhowe23 bmhowe23 Mar 10, 2025

Choose a reason for hiding this comment

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

A product_op is implicitly convertible to a sum_op, but not the other way around, so this change allows for the most flexibility in my opinion. (You are correct that what you had is the most backwards compatible by forcing it to return spin_op, but during the course of the 2704 PR, I had converted some things to spin_op_term (i.e. product_op), and my suggested change here allows seamless usage in both cases, I believe.)

Maybe I should've said: this allows for a good mix of backward compatibility and forward compatibility.

* the terms of the Apache License 2.0 which accompanies this distribution. *
******************************************************************************/

#pragma once
Copy link
Author

Choose a reason for hiding this comment

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

Adding this file just means people don't have to change their #include's. After all, they are still dealing with the spin_op data type.

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.

2 participants