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

Add Wigner plotting via CairoMakie #292

Merged
merged 13 commits into from
Dec 11, 2024

Conversation

LorenzoFioroni
Copy link
Contributor

@LorenzoFioroni LorenzoFioroni commented Nov 9, 2024

Checklist

  • Please read Contributor Covenant Code of Conduct
  • Any code changes were done in a way that does not break public API
  • Appropriate tests were added and tested locally by running: make test.
  • Any code changes should be julia formatted by running: make format.
  • All documents (in docs/ folder) related to code changes were updated and able to build locally by running make docs.

Description

This PR introduces plotting capabilities in QuantumToolbox (starting from the plot_wigner function implemented in QuTip).
To reduce the impact on loading time, I wrote an extension for the CairoMakie library, such that the necessary code is not loaded by default. Moreover, this allows extending the scheme to additional plotting libraries like PyPlot or Plots (although I haven't done it).

The plot_wigner function implements 2D and 3D projections of the Wigner and reproduces QuTip's output as faithfully as possible.

Related issues or PRs

fix #86

@ytdHuang
Copy link
Member

By the way, is it also possible to generate our logo with this function ?
I mean following the tutorial here: https://qutip.org/QuantumToolbox.jl/stable/tutorials/logo

@albertomercurio
Copy link
Member

I would say yes. Indeed, it would be a good thing to apply this function directly in the tutorial of the logo.

@LorenzoFioroni
Copy link
Contributor Author

It is straightforward to use this function to follow the logo tutorial up to where dissipation is included ("Introducing some decoherence"). However, I don't see an easy way to generate the full logo with the three julia colors, as that's not an heatmap anymore. In the tutorial you manually combine the colors to have a grid of RGB values displayed with image!.

Ideally, I would now add documentation and tests and fix a couple of minor details before pushing a new version. If you have any advice on how follow the tutorial till the end, though, I'd be happy to implement it.

@ytdHuang
Copy link
Member

@albertomercurio
any idea on this ?
I'm not familiar with Makie.jl...

@albertomercurio
Copy link
Member

As Lorenzo said, we can use plot_wigner in the logo tutorial up to a certain point. Then we use image instead of heatmap, because I create a custom image basically.

But still, it is a good idea to directly using it in the tutorial.

@LorenzoFioroni
Copy link
Contributor Author

Then sorry for the delay in bringing the PR to its final form. I hope I'll be able to finish adding tests and docs in the following days.

@ytdHuang
Copy link
Member

ytdHuang commented Dec 2, 2024

Then sorry for the delay in bringing the PR to its final form. I hope I'll be able to finish adding tests and docs in the following days.

It's okay, take your time.

@LorenzoFioroni LorenzoFioroni marked this pull request as ready for review December 9, 2024 19:06
@LorenzoFioroni LorenzoFioroni marked this pull request as draft December 9, 2024 19:21
@albertomercurio albertomercurio marked this pull request as ready for review December 9, 2024 21:00
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.

Please upload report for BASE (dev/visualization@519a191). Learn more about missing BASE report.

Files with missing lines Patch % Lines
ext/QuantumToolboxCairoMakieExt.jl 0.00% 39 Missing ⚠️
src/visualization.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##             dev/visualization    #292   +/-   ##
===================================================
  Coverage                     ?   9.23%           
===================================================
  Files                        ?      40           
  Lines                        ?    2641           
  Branches                     ?       0           
===================================================
  Hits                         ?     244           
  Misses                       ?    2397           
  Partials                     ?       0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ytdHuang ytdHuang changed the base branch from main to dev/visualization December 10, 2024 04:08
@ytdHuang
Copy link
Member

I have changed the target branch from main to dev/visualization.

I think the extension test structure and also the corresponding CI pipeline configs need to be modified, which I can do it afterwards.

Apart from that, I have a few more comments below.

src/visualization.jl Outdated Show resolved Hide resolved
@LorenzoFioroni
Copy link
Contributor Author

Thanks @ytdHuang for your input. I updated the docs to match your suggestions.
The only thing I left as it was is the use of :two_dim and :three_dim, as I don't think your suggestion would work (:(2d) is an Expr, not a Symbol - see above).
If you have advice on how to change it to 2d I'd be happy to do it. I also think it would be more natural.

@albertomercurio
Copy link
Member

I have changed the target branch from main to dev/visualization.

I think the extension test structure and also the corresponding CI pipeline configs need to be modified, which I can do it afterwards.

Apart from that, I have a few more comments below.

How exactly do you want to change the structure? I can also try to do it directly in this PR

@ytdHuang
Copy link
Member

ytdHuang commented Dec 11, 2024

I have changed the target branch from main to dev/visualization.
I think the extension test structure and also the corresponding CI pipeline configs need to be modified, which I can do it afterwards.
Apart from that, I have a few more comments below.

How exactly do you want to change the structure? I can also try to do it directly in this PR

It would be better to make this extension be an independent test group, so that the original core tests doesn't need to install CairoMakie.jl. This also need to change the settings of runtests CI pipeline.

I can do it afterwards.

@albertomercurio albertomercurio requested review from albertomercurio and ytdHuang and removed request for albertomercurio December 11, 2024 08:56
@ytdHuang ytdHuang merged commit 84c22ce into qutip:dev/visualization Dec 11, 2024
5 checks passed
@ytdHuang ytdHuang mentioned this pull request Dec 11, 2024
6 tasks
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.

Plot wigner
3 participants