Move functions to base class

[erick-xanadu]

Context: Some of these functions are needed in Catalyst. Since Catalyst does not use LM, but instead uses LightningDynamic, the least upper bound in the class hierarchy is StateVectorLQubit.

Description of the Change: Move these functions to StateVetctorLQubit.

Benefits: Code-reuse

Possible Drawbacks:

Related GitHub Issues:

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.33%. Comparing base (8f517d2) to head (0545954).
Report is 87 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (8f517d2) and HEAD (0545954). Click for more details.

HEAD has 63 uploads less than BASE

Flag	BASE (8f517d2)	HEAD (0545954)
	70	7

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #841      +/-   ##
==========================================
- Coverage   98.42%   92.33%   -6.10%     
==========================================
  Files         116       70      -46     
  Lines       18660    11211    -7449     
==========================================
- Hits        18367    10352    -8015     
- Misses        293      859     +566     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[maliasadi]

@erick-xanadu What was the issue with adding these methods to StateVectorLQubitDynamic? Any plan to get this support for LK in a followup PR?

[AmintorDusko]

The StateVectorLQubit has no explicit storage data type.
It might be tricky moving it here, or even unsafe.

[erick-xanadu]

@erick-xanadu What was the issue with adding these methods to StateVectorLQubitDynamic? Any plan to get this support for LK in a followup PR?

@maliasadi The idea was to reuse code. So moving these to the base class is only for increasing code reuse. LK's implementation is already in the base class.

[erick-xanadu]

The StateVectorLQubit has no explicit storage data type.
It might be tricky moving it here, or even unsafe.

@AmintorDusko Happy to close this ticket then. I'll leave it up for a bit of further discussion. The disadvantage is that we would copy these methods over. Right now for example, I am working on other two methods that will call these methods. And that means we would also have to copy these over in Catalyst.

[erick-xanadu]

@AmintorDusko alternatively, there could be another class between these two that implement these methods and Lightning Dynamic inherits from that one. Not sure how that would impact performance though. I know there is some static devirtualization being done through templates.

[vincentmr]

The StateVectorLQubit has no explicit storage data type.

What is preventing us from getting a pointer to the data and modifying it? All the gates are applied in this way already, so isn't it natural to do the StatePrep similarly?

[AmintorDusko]

The StateVectorLQubit has no explicit storage data type.
It might be tricky moving it here, or even unsafe.

@AmintorDusko Happy to close this ticket then. I'll leave it up for a bit of further discussion. The disadvantage is that we would copy these methods over. Right now for example, I am working on other two methods that will call these methods. And that means we would also have to copy these over in Catalyst.

Hey @erick-xanadu, feel free to proceed and get the team's opinion on that.
You don't need to close the PR.
I agree that copying is an advantage, especially in the maintenance context.
We can improve safety by including some checks in the methods, like checking the indices size and comparing it with the statevector size, before proceeding. What do you think?

[AmintorDusko]

The StateVectorLQubit has no explicit storage data type.

What is preventing us from getting a pointer to the data and modifying it? All the gates are applied in this way already, so isn't it natural to do the StatePrep similarly?

Nothing. See my comment addressing Eric.

[erick-xanadu]

We can improve safety by including some checks in the methods, like checking the indices size and comparing it with the statevector size, before proceeding. What do you think?

@AmintorDusko , added extra checks here: 7ddcc06

[vincentmr]

A couple suggestions, but I think it will be ready to merge after.

[vincentmr]

LGTM, thanks @erick-xanadu . Feel free to merge once the CI is green (I think you'll need to run make format).

[maliasadi]

Thanks @erick-xanadu! 🙌 Happy to approve after resolving my comment for setStateVector