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

ny_inner label misleading in topology schematic #2234

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

johnomotani
Copy link
Contributor

Qian Xia pointed out that the n_inner labels on the topology schematic are misleading. The label at upper outer divertor definitely should not be n_inner+1. This PR has a 'corrected' version of the figure, and also adds the .svg file (updated version of the one I got from @lukeeasy) in case someone wants to edit it in future (can be edited with inkscape).

I've changed n_inner to ny_inner because I think it's clearer.

I think the most consistent label for the outer boundaries is the number of the last grid point, i.e. nx-1, ny_inner-1, ny-1 instead of nx, ny_inner, ny, so I've updated that too.

@johnomotani
Copy link
Contributor Author

I've cancelled the CI (apart from readthedocs build) since this PR is only documentation (and I forgot to add a [skip ci] tag to the commit message).

@ZedThree
Copy link
Member

I think the filename should be topology_cross_section.* -- schematic is the following figure (see current next and this PR)

Is it possible to crop the SVG a bit too? Otherwise, looks good!

Most consistent label is the number of the last grid point, i.e. nx-1,
ny_inner-1, ny-1 instead of nx, ny_inner, ny.

Label at upper outer divertor definitely should not be ny_inner+1.

[skip ci]
@johnomotani
Copy link
Contributor Author

Thanks @ZedThree, I've made those changes. I've force-pushed the branch to avoid possibly adding more binary files to the history.

@ZedThree ZedThree merged commit 688bf74 into next Feb 17, 2021
@ZedThree ZedThree deleted the topology_schematic-fix branch February 17, 2021 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants