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

Adding __matmul__ to CircuitComponents #347

Merged
merged 34 commits into from
Feb 21, 2024
Merged

Adding __matmul__ to CircuitComponents #347

merged 34 commits into from
Feb 21, 2024

Conversation

SamFerracin
Copy link
Contributor

Context:
Adds matmul to circuit components.
Also changes/adds a bunch of stuff that is needed for it, such as:

  • Adds equality on wires and circuit components (so that tests become easier)
  • Changes Wires.__rshift__ into Wires.__matmul__, so that all rshift (matmul) in lab_dev mean the same thing, i.e. that they do (do not) add missing adjoints

Tests:
For the tests, it applies @ to sequences of states and gates, and compares the representation of the resulting component to those that one gets by implementing the same sequence with mrmustard.lab. Small differences found, which can be discussed offline

SamFerracin and others added 21 commits February 1, 2024 14:27
**Description of the Change:**
Using python built-in logger, this PR silences the annoying
`ComplexWarning`s coming from `tf.cast`.
Additionally, it gets rid of tensorflow's import messages.

---------

Co-authored-by: Filippo Miatto <ziofil@users.noreply.github.com>
**Context:**
In the latest MrM release, `scipy` was unpinned.
Syncing the `develop` branch with this latest release causes pytest
errors due to improper type handling on `scipy`'s side.

**Description of the Change:**
- Syncing `develop` with latest `main`
- Fixing scipy error by adding a `dtype` parameter to `math.sqrtm` so
that it never returns arrays of an unsupported type (specifically,
`complex256`)
- Updating dependencies
- Modifying the CHANGELOG in preparation for release

---------

Co-authored-by: zy <48225584+zeyueN@users.noreply.github.com>
**Context:**
The previous solution based on `logging` does not appear to work on
every OS
**Context:**
```
from mrmustard.lab import *
Vacuum(1)
```
Executing the code above triggers a long list of annoying warnings,
displayed between the recap of the state and the phase-space plot.

**Description of the Change:**
This PR removes the cause of the warnings, without changing the way that
the plot looks
**Context:**
```
probs = np.array([1e-6 for _ in range(300)])
results = [math.Categorical(probs, "") for _ in range(100)]
assert len(set(results)) > 1
```
When using the numpy backend, the code above fails: `results` contains
100 identical values.
This is because `math.Categorical` uses `np.random.multinomial`, which
assumes that the probabilities sum up to `1` (if not, the last
probability is increased such that the resulting probabilities sum up to
`1`)
**Context:** Add a new file triples.py in the physics module to include
all triples in Bargmann representations of the states and
transformations.
This PR will set up the basic physics for Bargmann objects.

**Description of the Change:**

**Benefits:**

**Possible Drawbacks:**

**Related GitHub Issues:**

---------

Co-authored-by: SamFerracin <samuele.ferracin@xanadu.ai>
Co-authored-by: SamFerracin <samuele.ferracin@gmail.com>
**Context:** Bug fix of the triples for attenuator

**Description of the Change:** c=1.0 for attenuator

**Benefits:**

**Possible Drawbacks:**

**Related GitHub Issues:**
@SamFerracin SamFerracin added the no changelog Pull request does not require a CHANGELOG entry label Feb 14, 2024
Copy link

codecov bot commented Feb 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (prototype-tn@07a114e). Click here to learn what that means.

Additional details and impacted files
@@               Coverage Diff               @@
##             prototype-tn     #347   +/-   ##
===============================================
  Coverage                ?   85.03%           
===============================================
  Files                   ?       74           
  Lines                   ?     5440           
  Branches                ?        0           
===============================================
  Hits                    ?     4626           
  Misses                  ?      814           
  Partials                ?        0           
Files Coverage Δ
mrmustard/lab/abstract/transformation.py 89.53% <100.00%> (ø)
mrmustard/lab_dev/__init__.py 100.00% <100.00%> (ø)
mrmustard/lab_dev/circuit_components.py 94.39% <100.00%> (ø)
mrmustard/lab_dev/simulator.py 98.03% <100.00%> (ø)
mrmustard/lab_dev/states.py 100.00% <100.00%> (ø)
mrmustard/lab_dev/transformations.py 98.00% <100.00%> (ø)
mrmustard/lab_dev/wires.py 100.00% <100.00%> (ø)
mrmustard/math/backend_manager.py 98.11% <100.00%> (ø)
mrmustard/math/backend_numpy.py 100.00% <ø> (ø)
mrmustard/math/backend_tensorflow.py 100.00% <ø> (ø)
... and 6 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07a114e...66aca97. Read the comment docs.

Copy link
Contributor

@sylviemonet sylviemonet left a comment

Choose a reason for hiding this comment

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

Nice PR!
I've checked the logic in side matmul which works for me!

Just leave an open question to discuss: if we do this matmul in simulator and with Fock in each step, would the idea to have the tensor network doesn't exist anymore?

mrmustard/lab_dev/states.py Show resolved Hide resolved
mrmustard/lab_dev/transformations.py Show resolved Hide resolved
mrmustard/physics/triples.py Outdated Show resolved Hide resolved
…epresentation docs to make them consistent (#341)

**Context:**
Fock and ArrayAnsatz, docs and tests

---------

Co-authored-by: Filippo Miatto <miatto@gmail.com>
Co-authored-by: Filippo Miatto <ziofil@users.noreply.github.com>
Co-authored-by: Yuan <16817699+sylviemonet@users.noreply.github.com>
Co-authored-by: Sebastián Duque Mesa <675763+sduquemesa@users.noreply.github.com>
Co-authored-by: zy <48225584+zeyueN@users.noreply.github.com>
@SamFerracin
Copy link
Contributor Author

SamFerracin commented Feb 15, 2024

Nice PR! I've checked the logic in side matmul which works for me!

Just leave an open question to discuss: if we do this matmul in simulator and with Fock in each step, would the idea to have the tensor network doesn't exist anymore?

@sylviemonet the tensor network idea still exists, but it is orthogonal to the matmul/rshift.
When we specify a path for a circuit, the setter will take care to check that the path is valid for the given circuit based on the connectivity of the components. This way, the check is done only once and outside of the simulation (remember that I may want to simulate the same circuit 1000 times)

@sylviemonet sylviemonet self-requested a review February 20, 2024 19:40
Copy link
Contributor

@sylviemonet sylviemonet left a comment

Choose a reason for hiding this comment

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

It looks good to me now!

sylviemonet and others added 3 commits February 21, 2024 12:17
**Context:** There are three bugs fixed in this PR.

1. the projection onto the Vacuum state will use the generaldyne where
the projection function is not correct.
For example,
`from mrmustard.lab import *;Vacuum(3) << Vacuum(3)`
The projection in the example above is not 1.0, which should be 1.0.
This has been fixed by changing the functions inside generaldyne.
3. add more text to explain about complex_gaussian_integral and other
function in bargmann.py.
4. fix the problems that the state will be reassigned during the running
of the circuit.
For example,
`from mrmustard.lab import *;
vac0 = Vacuum(1);
d1 = Dgate(0.1, 0.1);
vac0 >> d1;
vac0.bargmann()`

the state `vac0` state will be modified after going through the first
Dgate, which is not the behaviour we want.

**Description of the Change:**

**Benefits:**

**Possible Drawbacks:**

**Related GitHub Issues:**

---------

Co-authored-by: Filippo Miatto <ziofil@users.noreply.github.com>
Co-authored-by: SamFerracin <samuele.ferracin@xanadu.ai>
…tests (#348)

**Context:** Because of the tests were `np.allclose(b, 0)`, we haven't
tested the length of the b vector just the values. Hence we have
mistakes in the length of b Vector of rotation gate, thermal state and
attenuator/amplifier channels.
In this PR, we fix the b vectors of them and fix the tests with respect
to the shape of the vector it should be.
We also fix the type hint for the functions.

**Description of the Change:**

**Benefits:**

**Possible Drawbacks:**

**Related GitHub Issues:**

---------

Co-authored-by: SamFerracin <samuele.ferracin@gmail.com>
mrmustard/lab_dev/wires.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@ziofil ziofil left a comment

Choose a reason for hiding this comment

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

LGTM!

@SamFerracin SamFerracin merged commit 4d53347 into prototype-tn Feb 21, 2024
7 checks passed
@SamFerracin SamFerracin deleted the matmul-cc branch February 21, 2024 22:19
@SamFerracin SamFerracin restored the matmul-cc branch February 27, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Pull request does not require a CHANGELOG entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants