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.

Sign up for GitHub

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

Jump to bottom

increasing fat_bands_slide/ removing spline #883

Merged
merged 4 commits into from
Oct 23, 2024
Merged

increasing fat_bands_slide/ removing spline #883

merged 4 commits into from
Oct 23, 2024

Conversation

AndresOrtegaGuerrero
Copy link
Member

@AndresOrtegaGuerrero AndresOrtegaGuerrero commented Oct 22, 2024 •
edited
Loading

This PR address #875

We are increasing the max value of fat bands to 2.0
We are removing the spline option
We additionally are switing to plotly.graph_objects.Scattergl since is lighter and it shows a better performance response ( > 80 atoms)

@codecov Codecov
Copy link

codecov bot commented Oct 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.15%. Comparing base (30ea955) to head (8cff169).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #883   +/-   ##
=======================================
  Coverage   67.15%   67.15%           
=======================================
  Files          51       51           
  Lines        4707     4707           
=======================================
  Hits         3161     3161           
  Misses       1546     1546
Flag Coverage Δ
python-3.11 67.15% <100.00%> (ø)
python-3.9 67.18% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@superstar54
Copy link
Member

Hi Andres, thanks for the PR.

I noticed there is a little difference at the bottom of the bands plot:
old one:
Screenshot from 2024-10-23 10-15-10
new one
Screenshot from 2024-10-23 10-16-29

Is it acceptable that there is not lines there?

@AndresOrtegaGuerrero
Copy link
Member Author

@superstar54 Yeah I noticed this , emm it might be due to using Scattergl , for me it is , actually it might less confusing that it just shows a bar, i check and the behaviour of the limiting bar still does the same work. Since i am observing way better performance for big system , this is acceptable for me. Also because in the past some people were confused that the display of the lines so i see it as a win/win :D

superstar54
superstar54 approved these changes Oct 23, 2024
View reviewed changes
Copy link
Member

@superstar54 superstar54 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

@AndresOrtegaGuerrero AndresOrtegaGuerrero merged commit b0cfdbd into main Oct 23, 2024
12 checks passed
@AndresOrtegaGuerrero AndresOrtegaGuerrero deleted the bandspdos_widget_update branch October 23, 2024 08:56
edan-bainglass pushed a commit to edan-bainglass/aiidalab-qe that referenced this pull request Oct 24, 2024
@AndresOrtegaGuerrero @edan-bainglass
increasing fat_bands_slide/ removing spline (aiidalab#883)
a0b6d06
@AndresOrtegaGuerrero AndresOrtegaGuerrero linked an issue Oct 24, 2024 that may be closed by this pull request
Check if performance of (PDOS and Bands plot) can be improved for large systems #900
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@superstar54 superstar54 superstar54 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Check if performance of (PDOS and Bands plot) can be improved for large systems
2 participants
@AndresOrtegaGuerrero @superstar54