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

[Runtime] Allow runtime to be compiled in ILP32 mode. #230

Merged
merged 7 commits into from
Aug 9, 2023

Conversation

erick-xanadu
Copy link
Contributor

@erick-xanadu erick-xanadu commented Aug 8, 2023

Context: Error seen when compiling on macOS arm64

Description of the Change: Remove reinterpret_cast

Benefits: Can compile with ILP32 mode (used by macOS)

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #230 (d452dd3) into main (74e59e1) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #230      +/-   ##
==========================================
- Coverage   99.22%   99.22%   -0.01%     
==========================================
  Files          37       37              
  Lines        6699     6697       -2     
  Branches      340      340              
==========================================
- Hits         6647     6645       -2     
  Misses         29       29              
  Partials       23       23              
Files Changed Coverage Δ
...time/lib/backend/lightning/LightningObsManager.hpp 100.00% <100.00%> (ø)
runtime/tests/Test_LightningCoreQIS.cpp 100.00% <100.00%> (ø)

@erick-xanadu erick-xanadu marked this pull request as ready for review August 8, 2023 22:53
@erick-xanadu
Copy link
Contributor Author

@maliasadi I think we don't need a change log for this one, what do you think?

@mlxd
Copy link
Member

mlxd commented Aug 9, 2023

@maliasadi I think we don't need a change log for this one, what do you think?

Always good to add a changelog :)

Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

Thanks @erick-xanadu! Don't forget to check other devices (lightning-kokkos, openqasm) too.

@maliasadi
Copy link
Member

@maliasadi I think we don't need a change log for this one, what do you think?

I think it's good to add this for the future reference.

@erick-xanadu
Copy link
Contributor Author

Don't forget to check other devices (lightning-kokkos, openqasm) too.

What do you mean here? Wouldn't the CI/CD tests cover these?

@maliasadi
Copy link
Member

maliasadi commented Aug 9, 2023

What do you mean here? Wouldn't the CI/CD tests cover these?

You may need to update other devices Obs Manager classes after building catalyst with those backend devices on our local MacOS arm64 device.

CI/CD checks all the devices but not on MacOS arm64 yet.

@erick-xanadu
Copy link
Contributor Author

Got it. They all unconditionally get compiled, correct? If that's the case, then since this is a compile time error, this patch should be sufficient.

@maliasadi
Copy link
Member

You still need to check/update LightningKokkosObsManager and maybe OpenQASMObsManager. Did you check those devices on the machine?

doc/changelog.md Outdated Show resolved Hide resolved
@erick-xanadu erick-xanadu changed the title [Runtime] Remove reinterpret cast. [Runtime] Allow runtime to be compiled in ILP32 mode. Aug 9, 2023
Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

💯

@erick-xanadu erick-xanadu merged commit 78ff549 into main Aug 9, 2023
22 checks passed
@erick-xanadu erick-xanadu deleted the eochoa/2023-08-08/remove-reinterpret-cast branch August 9, 2023 17:13
@maliasadi
Copy link
Member

[sc-43392]

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

3 participants