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

increase min cutoff for visualize2d/3d to 50 #463

Merged
merged 8 commits into from
Aug 20, 2024
Merged

Conversation

ziofil
Copy link
Collaborator

@ziofil ziofil commented Aug 15, 2024

Visualize2d/3d can show a wrong image when plotting non-DM wigner functions, such as the parity operator. This PR guarantees a minimum fock cutoff of 50 for the wigner plot, which solves most of these issues.

Before:

Screenshot 2024-08-15 at 11 56 02 AM

After:

Screenshot 2024-08-15 at 11 56 32 AM

Also 3d (after):
Screenshot 2024-08-15 at 11 57 57 AM

@ziofil ziofil changed the title increase min cutoff for visualize2d to 50 increase min cutoff for visualize2d/3d to 50 Aug 15, 2024
@ziofil ziofil added the no changelog Pull request does not require a CHANGELOG entry label Aug 15, 2024
@ziofil ziofil requested a review from timmysilv August 15, 2024 18:58
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.22%. Comparing base (4ac64c1) to head (d38ee64).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #463      +/-   ##
===========================================
+ Coverage    89.21%   89.22%   +0.01%     
===========================================
  Files          102      102              
  Lines         7194     7201       +7     
===========================================
+ Hits          6418     6425       +7     
  Misses         776      776              
Files Coverage Δ
mrmustard/lab_dev/circuit_components.py 98.38% <100.00%> (+0.02%) ⬆️
mrmustard/lab_dev/states/base.py 96.93% <100.00%> (+0.01%) ⬆️

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 4ac64c1...d38ee64. Read the comment docs.

@timmysilv
Copy link
Collaborator

i can't speak to whether there are cases where this minimum doesn't really make sense, but I am guessing you can - do any instances come to mind where we don't want this minimum, and if so, can we use the new kwarg in those cases? otherwise, this looks good to me

@ziofil
Copy link
Collaborator Author

ziofil commented Aug 19, 2024

i can't speak to whether there are cases where this minimum doesn't really make sense, but I am guessing you can - do any instances come to mind where we don't want this minimum, and if so, can we use the new kwarg in those cases? otherwise, this looks good to me

We'll replace the discretized method with calling the new ansatz as soon as it's available. In the meantime this is good enough to not incur in odd results like you'd have with too small cutoff.

@ziofil ziofil merged commit 7255f25 into develop Aug 20, 2024
7 checks passed
@ziofil ziofil deleted the visualize2d_cutoff branch August 20, 2024 04:44
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.

2 participants