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

Organize arguments of functions and apply colorblind_safe option #2170

Merged
merged 58 commits into from Jul 7, 2023

Conversation

tamakoshi2001
Copy link
Contributor

@tamakoshi2001 tamakoshi2001 commented Jun 2, 2023

Checklist
Thank you for contributing to QuTiP! Please make sure you have finished the following tasks before opening the PR.

  • Please read Contributing to QuTiP Development
  • Contributions to qutip should follow the pep8 style.
    You can use pycodestyle to check your code automatically
  • Please add tests to cover your changes if applicable.
  • If the behavior of the code has changed or new feature has been added, please also update the documentation in the doc folder, and the notebook. Feel free to ask if you are not sure.
  • Include the changelog in a file named: doc/changes/<PR number>.<type> 'type' can be one of the following: feature, bugfix, doc, removal, misc, or deprecation (see here for more information).

Delete this checklist after you have completed all the tasks. If you have not finished them all, you can also open a Draft Pull Request to let the others know this on-going work and keep this checklist in the PR description.

Description
This is part of the Google Summer of Code project. My contribution consists of two parts.
One part is organizing arguments of functions in visualization.py. This change helps users to set arguments without referencing documents frequently.
For example, hinton does not have figsize, but other functions such as plot_energy_level do. Also, the order of the arguments varies from function to function, so the user had to see the official document.
I addressed these issues by making the following modifications
First, arguments that many functions should have in common, such as fig and ax, are now keyword arguments.
Second, the order of the arguments was aligned as required arguments, function-specific ones, and common ones, making it easier for the user to remember and use.

# before
hinton(rho, xlabels=None, ylabels=None, title=None, 
       ax=None, cmap=None, label_top=True, color_style='scaled')
# after
hinton(rho, x_basis=None, y_basis=None, color_style="scaled",
       label_top=True, *, cmap=None, colorbar=True, fig=None, ax=None)

The other is applying colorblind_safe to them. QuTiP has colorblind_safe as a setting option, but most functions did not change their color corresponding to colorblind_safe. Now, they change their plot colors when you change the colorblind_safe option and you can change color if you do not like the default color.
For example, when quite.settings.colorblind_safe=False, a plot by plot_spin_distribution is the left plot. On the other hand, when quite.settings.colorblind_safe=False, it outputs the right plot in the different colormap (cividis). This is perceptually uniform. Of course, you can change the colormap as you like.
colorblind_safe

@coveralls
Copy link

coveralls commented Jun 2, 2023

Coverage Status

coverage: 78.18% (+0.07%) from 78.106% when pulling e98bb7b on tamakoshi2001:GSoC_project6_week1 into f66b8f2 on qutip:master.

@tamakoshi2001 tamakoshi2001 changed the title add class Color (draft PR for Google Summer of Code 2023 project6) add class Color and merge spin distribution functions(draft PR for Google Summer of Code 2023 project6) Jun 4, 2023
@tamakoshi2001
Copy link
Contributor Author

I combined spin distribution functions for 2d and 3d into one. Those two will be deleted.

@tamakoshi2001
Copy link
Contributor Author

I added new_hinton. The arguments in new_hinton are more organized than those in hinton. It will replace hinton.

qutip/color.py Outdated Show resolved Hide resolved
qutip/color.py Outdated Show resolved Hide resolved
qutip/visualization.py Outdated Show resolved Hide resolved
qutip/visualization.py Outdated Show resolved Hide resolved
qutip/visualization.py Outdated Show resolved Hide resolved
@tamakoshi2001
Copy link
Contributor Author

@Ericgig Hi. I changed hinton, matrix_histogram_complex, and plot_energy_level this weekend. Could you let me know if the selection of arguments is good? Thank you. Here is a sample use.
https://drive.google.com/file/d/1LSNLWW2FBhX4PeMXJTxSN6Qgb-G6_TXn/view?usp=sharing

qutip/visualization.py Outdated Show resolved Hide resolved
qutip/visualization.py Outdated Show resolved Hide resolved
@tamakoshi2001
Copy link
Contributor Author

tamakoshi2001 commented Jun 19, 2023

Hi. I worked on
hinton,

sphereplot,
matrix_histogram_complex
plot_energy_levels,
plot_fock_distribution,
plot_wigner,
plot_expectation_values,
plot_spin_distribution (cmap will be changed)
I would like you to see if they are ok. Here is the example use; https://drive.google.com/file/d/1rGcTEWz2c9-wRnQSWSVyt5SfVTVaTkvl/view?usp=sharing.

@tamakoshi2001
Copy link
Contributor Author

I will work on
plot_wigner_sphere,
matrix_histogram,
inner functions,
deprecated functions (e.g. energy level diagram),
cmap on qubism and schmidt,
documents

@tamakoshi2001
Copy link
Contributor Author

Other than doing the changes you suggested, I updated the description and deleted all deprecated functions.

@tamakoshi2001
Copy link
Contributor Author

@Ericgig Hi. I have some questions about your suggestions above. Could you answer them?

@tamakoshi2001
Copy link
Contributor Author

@Ericgig Hi. I made a few changes.

@tamakoshi2001
Copy link
Contributor Author

@Ericgig Could you review the code?

Comment on lines 843 to 846
try:
x_basis, y_basis = _default_cb_labels(x_basis, y_basis, M)
except Exception:
pass
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a try, except clause here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That code makes bra and ket like labels by using dimensions of Qobj. If it does not exist, when users want to see superoperators, the code outputs an error because of their dimensions. I believe showing plot is more important than adding labels.
superoperator.pdf

Copy link
Member

Choose a reason for hiding this comment

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

Dimensions exist for all Qobj including superoperators.
If _default_cb_labels does not support superoperator, we could add that support or use

Suggested change
try:
x_basis, y_basis = _default_cb_labels(x_basis, y_basis, M)
except Exception:
pass
if not M.issuper:
x_basis, y_basis = _default_cb_labels(x_basis, y_basis, M)

to make clear this is a special case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi. I do not know that dimensions exist for all Qobj. I used the shape of Qobj and the code should work for all Qobj now. I deleted _default_cb_labels because this is made to follow pep8 and we no longer need it.

Copy link
Member

@Ericgig Ericgig left a comment

Choose a reason for hiding this comment

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

Look good, I will play with it a little more tomorrow before finishing the review.

Copy link
Member

@Ericgig Ericgig left a comment

Choose a reason for hiding this comment

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

Look fine, thank you

@Ericgig Ericgig merged commit 7cf468c into qutip:master Jul 7, 2023
12 checks passed
@tamakoshi2001 tamakoshi2001 changed the title Organize arguments of functions and apply colorblind_safe option(draft PR for Google Summer of Code 2023 project6) Organize arguments of functions and apply colorblind_safe option Aug 24, 2023
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.

None yet

3 participants