XanaduAI / MrMustard

A differentiable bridge between phase space and Fock space
https://mrmustard.readthedocs.io/
Apache License 2.0
78 stars 27 forks source link

to_bargmann() original bargmann data bug #486

Closed apchytr closed 2 months ago

apchytr commented 2 months ago

Context: currently on develop to_bargmann is not checking if original bargmann data exists leading to issues such as

state = SqueezedVacuum([0], r=1.0)
dgate = Dgate([0], x=10.0).to_fock(150)
state >> dgate

running out of memory.

Description of the Change: to_bargmann now checks for original bargmann data. Removed a single line that has original_fock_data as that is not used anywhere. Moved some things to ansatz. __truediv__ and __mul__ can update the original bargmann data as long as other is a scalar. _original_bargmann_data is renamed to _original_abc_data. Some Representation cleanup removing duplicate code.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 99.14530% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.60%. Comparing base (06c000b) to head (adff94a). Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
mrmustard/physics/ansatze.py 98.95% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #486 +/- ## ======================================== Coverage 89.60% 89.60% ======================================== Files 102 102 Lines 7415 7419 +4 ======================================== + Hits 6644 6648 +4 Misses 771 771 ``` | [Files with missing lines](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) | Coverage Δ | | |---|---|---| | [mrmustard/lab\_dev/circuit\_components.py](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?src=pr&el=tree&filepath=mrmustard%2Flab_dev%2Fcircuit_components.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-bXJtdXN0YXJkL2xhYl9kZXYvY2lyY3VpdF9jb21wb25lbnRzLnB5) | `99.09% <100.00%> (+<0.01%)` | :arrow_up: | | [mrmustard/physics/representations.py](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?src=pr&el=tree&filepath=mrmustard%2Fphysics%2Frepresentations.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-bXJtdXN0YXJkL3BoeXNpY3MvcmVwcmVzZW50YXRpb25zLnB5) | `98.58% <100.00%> (-0.07%)` | :arrow_down: | | [mrmustard/physics/ansatze.py](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?src=pr&el=tree&filepath=mrmustard%2Fphysics%2Fansatze.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-bXJtdXN0YXJkL3BoeXNpY3MvYW5zYXR6ZS5weQ==) | `97.27% <98.95%> (+0.08%)` | :arrow_up: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). Last update [06c000b...adff94a](https://app.codecov.io/gh/XanaduAI/MrMustard/pull/486?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI).
timmysilv commented 2 months ago

looks good! I'm asking this more for personal clarification than anything else: what value do we gain from this separation of representations and ansatze? Is it in anticipation for future things? I'm wondering this because I see that this separation is the reason you lowered the _original data to the ansatz itself.

apchytr commented 2 months ago

looks good! I'm asking this more for personal clarification than anything else: what value do we gain from this separation of representations and ansatze? Is it in anticipation for future things? I'm wondering this because I see that this separation is the reason you lowered the _original data to the ansatz itself.

So I think the original design was that we would have different types of ansatz but I believe as we continue to develop lab_dev its becoming more and more evident that this separation is unnecessary. The only thing I could think of is that maybe codefactor will give us issues if we merge the two.