-
Notifications
You must be signed in to change notification settings - Fork 39
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
Remove SVDynamic #999
Remove SVDynamic #999
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #999 +/- ##
==========================================
- Coverage 97.74% 91.74% -6.01%
==========================================
Files 231 176 -55
Lines 36767 24507 -12260
==========================================
- Hits 35938 22483 -13455
- Misses 829 2024 +1195 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @rauletorresc! 🥳
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question for Ali:
Context: When the LQ plugin was migrated from Catalyst to Lightning, the
StateVectorLQDynamic
class was also migrated. Given the fact that we haveStateVectorLQManaged
, we can get rid ofStateVectorLQDynamic
.Description of the Change: Replace the uses of
StateVectorLQDynamic
withStateVectorLQManaged
. I basically reused the code from the LK simulator and removed the calls to theStateVectorLQDynamic
related methods. Of course, I had to usestd::complex
instead ofkokkos::complex
.Benefits: Using the same underlying class for LG, LK and LQ.
NOTE: I have deleted the only two tests that were failing:
State(...)
showed a size of 2 for the given state and a size of 4 for the device state, which made the assertion fail.It seems to me these two tests do not comply with the way
StateVectorLQManaged
is structured but let the discussion resolve it.[sc-77689]