-
Notifications
You must be signed in to change notification settings - Fork 4
Add reverse_current to tokamak_example.py #206
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
base: master
Are you sure you want to change the base?
Conversation
johnomotani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @mikekryjak up to you if you want to address the comment.
examples/tokamak/tokamak_example.py
Outdated
| parser.add_argument("--ny", type=int, default=65) | ||
| parser.add_argument("--np", "--number-of-processors", type=int, default=-1) | ||
| parser.add_argument("--no-plot", action="store_true", default=False) | ||
| parser.add_argument("--reverse-current", action="store_true", default=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this example only works properly with the current reversed, maybe we want to default to doing this, so make the command line flag --no-reverse-current and reverse the current unless the user passes this flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to break anything else that this example is used for. If you can't think of anything, then happy to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know this example isn't used in production for anything, or for tests (apart from a test that it runs at all), so I think it should be fine. Anyway, better to have a more usable solution than stick to the existing one just for historical consistency - something like this I think is low risk and if it affects anyone they can raise an issue.
|
Added a new commit, mostly to trigger the CI jobs (which had been suspended due to inactivity on the repo). Added the option to manually trigger CI, which would have been useful now! |
…ject/hypnotoad into examples-reverse-current
|
@johnomotani I fixed the |
tokamak_example.pyis a precious resource because it can generate example grids of all topologies. We need this for all kinds of tests in Hermes-3, xBOUT and xHermes.Unfortunately, the COCOS convention is such that you get warnings about negative
Jdue to a negativedxwhen you run with the grids.This PR adds the flag--reverse-currenttotokamak_example.pywhich passes it to the yaml input file. This flips thepsidirection and allows it to be run.This PR changes the default current direction in the YAML files for the examples. There is now a new flag
--original-cocosto disablereverse_current. There is also a new flag called--no-guardsto generate a grid without guard cells.doc/whats-new.mdwith a summary of the changes