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

Polygonal 2D poloidal plots #280

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Polygonal 2D poloidal plots #280

wants to merge 30 commits into from

Conversation

mikekryjak
Copy link
Collaborator

@mikekryjak mikekryjak commented Mar 1, 2023

xBOUT's default pcolormesh() method creates plotting artifacts around the X-point due to having to plot the grid on a per-region basis and not having corner information.

This new method uses a different approach, and is more like what UEDGE, SOLEDGE2D and SOLPS users do. Using coordinates of cell centres and cell corners, a Matplotlib Polygon is created for each cell independently and put into a PatchCollection . With each cell being independently plotted, there are no issues arising from the poloidal ordering of the cells or from the X-point.

The polygon plotting routine was adapted from this PR which was itself based on a UEDGE routine. The cell corners were obtained using this Hypnotoad PR. It adds a new plot method accessed by da.bout.polygon() which creates fast, pretty 2D plots without visual artefacts around the X-point.

  • Add facility to read the cell corners into dataset coords in geometry.py
  • Create plot2d_polygon in plotfuncs.py
  • Test on single null
  • Make sure it works when plotting with guard cells as well as without

Decided to not integrate it with plot2d_wrapper as polygon doesn't plot on a region basis and does not wrap Xarray plot routines.

There is an additional improvement where I fixed the colorbar to be always exactly the height of the plot using a hack from https://joseph-long.com/writing/colorbars/.

I also changed the default style - I disabled the targets and limiter hatching and made the separatrix a continuous thin white line. I personally think this is a cleaner look, but I understand this is personal preference. If anyone has strong feelings about this I'm happy to make the previous look the default.

Example output:
image

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

Merging #280 (5391d64) into master (212b407) will decrease coverage by 1.24%.
The diff coverage is 4.76%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
- Coverage   68.94%   67.71%   -1.24%     
==========================================
  Files          15       15              
  Lines        3214     3277      +63     
  Branches      792      808      +16     
==========================================
+ Hits         2216     2219       +3     
- Misses        734      793      +59     
- Partials      264      265       +1     
Impacted Files Coverage Δ
xbout/geometries.py 68.87% <0.00%> (-1.08%) ⬇️
xbout/plotting/plotfuncs.py 16.19% <1.75%> (-2.48%) ⬇️
xbout/boutdataarray.py 78.20% <50.00%> (-0.16%) ⬇️
xbout/region.py 83.59% <100.00%> (+0.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pep8speaks
Copy link

pep8speaks commented Mar 22, 2024

Hello @mikekryjak! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 886:89: E501 line too long (90 > 88 characters)
Line 894:89: E501 line too long (103 > 88 characters)
Line 914:89: E501 line too long (90 > 88 characters)
Line 935:23: E711 comparison to None should be 'if cond is None:'
Line 938:22: E711 comparison to None should be 'if cond is not None:'

Line 124:89: E501 line too long (116 > 88 characters)

Line 223:37: E701 multiple statements on one line (colon)

Line 475:5: E704 multiple statements on one line (def)
Line 625:5: E704 multiple statements on one line (def)

Comment last updated at 2024-05-28 15:22:58 UTC

@mikekryjak mikekryjak marked this pull request as ready for review March 25, 2024 12:41
@mikekryjak
Copy link
Collaborator Author

This is ready for review & merge

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