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

No longer in-place modify input to apply #474

Merged
merged 4 commits into from
Aug 18, 2023

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Aug 17, 2023

In Pennylane PR #4485, tape.operations will no longer be the sum of tape._prep and tape._ops.

This means any modification of the tape.operations list will mutate the tape itself, instead of a shallow copy of its contents.

A basic example of this can be shown with the below list manipulations:

>>> a = [1]
>>> b = [2]
>>> c = a+b
>>> c[0] = 5
>>> c
[5, 2]
>>> a
[1]
>>> d = a
>>> d[0] = 5
>>> a
[5]

Long story short, we do not want to in-place modify the operations list anymore. So instead of using del operations[0], I simply select out the contents from index one onward.

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@albi3ro albi3ro requested a review from a team August 17, 2023 21:26
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #474 (ca217cb) into merge/monorepo (7b6f230) will decrease coverage by 0.43%.
Report is 7 commits behind head on merge/monorepo.
The diff coverage is 100.00%.

@@                Coverage Diff                 @@
##           merge/monorepo     #474      +/-   ##
==================================================
- Coverage           97.21%   96.79%   -0.43%     
==================================================
  Files                 141      142       +1     
  Lines               16280    16420     +140     
==================================================
+ Hits                15827    15894      +67     
- Misses                453      526      +73     
Files Changed Coverage Δ
...ane_lightning/lightning_kokkos/lightning_kokkos.py 93.97% <100.00%> (ø)
...ylane_lightning/lightning_qubit/lightning_qubit.py 92.54% <100.00%> (ø)

... and 20 files with indirect coverage changes

Copy link
Contributor

@vincentmr vincentmr left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @albi3ro !

@vincentmr
Copy link
Contributor

@albi3ro Don't you also want this fixed on master?

Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Hi @albi3ro, this is a sensible update.
Thank you for that.

@AmintorDusko
Copy link
Contributor

@albi3ro, feel free to merge it. You can ignore the CI fails.

@albi3ro
Copy link
Contributor Author

albi3ro commented Aug 18, 2023

@albi3ro Don't you also want this fixed on master?

I can open up a PR to master if need be.

@vincentmr
Copy link
Contributor

I can open up a PR to master if need be.

I can do it too, I meant do you need it out asap (for example if this is a blocker in some CI pulling lightning@master)? Because merge/monorepo will take some more time.

@mlxd
Copy link
Member

mlxd commented Aug 18, 2023

I can open up a PR to master if need be.

I can do it too, I meant do you need it out asap (for example if this is a blocker in some CI pulling lightning@master)? Because merge/monorepo will take some more time.

Let's get it into master now too then, as the CI is likely going to be heavily hit today, and having the most recent version will be worthwhile.

@vincentmr
Copy link
Contributor

I can open up a PR to master if need be.

I can do it too, I meant do you need it out asap (for example if this is a blocker in some CI pulling lightning@master)? Because merge/monorepo will take some more time.

Let's get it into master now too then, as the CI is likely going to be heavily hit today, and having the most recent version will be worthwhile.

Will take care of it.

@AmintorDusko AmintorDusko merged commit 7780e6f into merge/monorepo Aug 18, 2023
@AmintorDusko AmintorDusko deleted the fix-operation-list-mutation branch August 18, 2023 18:52
mlxd pushed a commit that referenced this pull request Aug 21, 2023
…ved repository (#472)

* Create a new single repository structure and merge Lightning Qubit and Lightning Kokkos backends

Co-authored-by: vincentmr <[email protected]>

* remove benchmarks CI actions for now

* update _version.py location on actions

* update changelog

* update version

* update action python version

* pleasing CodeFactor

* remove doc/code/api

* ignore doc/code/api

* update wheels build actions

* add coverage test

* add developer tools to check coverage

* update changelog

* fix broken actions

* update test linux

* update upload pypi action name

* update testing configuration strategy

* fix tests without binary

* fix tests without binary

* fix tests without binary

* build wheels for LKokkos but do not push to pypi

* expand QuantumScriptSerializer testing

* pin pennylane back to what should be

* now, really pinning pennylane back to what should be

* pin pennylane back

* break CIs

* unbreak my CIs... 🎶

* adjust test precision

* uninstall pennylane-lightning before testing LightningKokkos

* fix wheels

* change uninstall pennylane-lightning to install new pennylane-lightning without binaries

* fix if statement

* No longer in-place modify input to `apply` (#474)

* stop mutating input to apply

* changelog

* Reformat.

---------

Co-authored-by: Vincent Michaud-Rioux <[email protected]>
Co-authored-by: AmintorDusko <[email protected]>

* include jax data type

* configure jax precision and test tolerance properly

* format

* update dev version

---------

Co-authored-by: vincentmr <[email protected]>
Co-authored-by: Christina Lee <[email protected]>
AmintorDusko added a commit that referenced this pull request Aug 24, 2023
* Create a new single repository structure and merge Lightning Qubit and Lightning Kokkos backends

* remove benchmarks CI actions for now

* update _version.py location on actions

* update changelog

* update version

* update action python version

* pleasing CodeFactor

* remove doc/code/api

* ignore doc/code/api

* update wheels build actions

* add coverage test

* add developer tools to check coverage

* update changelog

* fix broken actions

* update test linux

* update upload pypi action name

* update testing configuration strategy

* fix tests without binary

* fix tests without binary

* fix tests without binary

* build wheels for LKokkos but do not push to pypi

* expand QuantumScriptSerializer testing

* pin pennylane back to what should be

* now, really pinning pennylane back to what should be

* docs initial push

* pin pennylane back

* add lighning_qubit and lightning_kokkos packages to the automodapi

* unbreak my CIs... 🎶

* adjust test precision

* uninstall pennylane-lightning before testing LightningKokkos

* fix wheels

* change uninstall pennylane-lightning to install new pennylane-lightning without binaries

* fix if statement

* No longer in-place modify input to `apply` (#474)

* stop mutating input to apply

* changelog

* Reformat.

* include jax data type

* configure jax precision and test tolerance properly

* format

* update dev version

* update avx docs

* add Lightning image

* add Lightning image file

* update Plugin name to Lightning

* Fix merge

* more fixing to merge

* Auto update version

* fix bin files

* fix lightning kokkos docs

* install the lightning_kokkos backend

* remove about title

* Update Python on readthedocs action

* fix PYPI Readme rendering

* Auto update version

* Content and structural changes coming from review suggestions

* update lightning kokkos installation guidelines

* Update lists of 'Supported operations and observables' in device docs.

* Fix Exp Prod SProd Sum links in doc.

* review suggestions and some small fixes

* Fix couple formatting typos.

* Add changelog and more typo fixes

---------

Co-authored-by: Vincent Michaud-Rioux <[email protected]>
Co-authored-by: AmintorDusko <[email protected]>
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.

4 participants