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

Handle single basis in embed #162

Closed
wants to merge 1 commit into from

Conversation

a-eghrari
Copy link

This commit addresses the issue where the embed function fails when it receives a single basis instead of a composite basis as input. #159

This commit addresses the issue where the `embed` function fails when it receives a single basis instead of a composite basis as input.
@Krastanov
Copy link
Collaborator

I only now noticed I have not enabled the tests for your PR. Apologies for that, Github does not run tests automatically on first PR as a security precaution.

Running the tests reveals that it would be best to make this change in the foundational package QuantumInterface.jl, otherwise we have a couple of issues:

  • This change will not be picked up by other libraries that might benefit from it
  • It causes type piracy given that Basis is not owned by this package, but it is owned by QuantumInterface.jl

Could you consider making the contribution to that other package?

Also, to avoid ambiguity, it should probably be defined for embed(basis::Basis, indices, ops::AbstractOperator) (i.e. there should be a type assert on the last argument).

Thank you again for the contribution. Really sorry for making you wait so long for a review.

@Krastanov
Copy link
Collaborator

I see you have made a PR to QuantumInterface at qojulia/QuantumInterface.jl#24
Thank you! I will close this PR here and proceed there.

@Krastanov Krastanov closed this Jun 13, 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.

2 participants