Add CLI for plotting CCE output data #6011
Conversation
| callback=_parse_modes, | ||
| required=True, | ||
| help=( | ||
| "Which mode to plot. Specified as 'l,m'. Will plot both real and" |
There was a problem hiding this comment.
I personally don't like using nargs=2 for click.options (for click.arguments they are good). It feels more natural to me to have comma separated value (maybe it's because there isn't a space so it feels like one option rather than two).
There was a problem hiding this comment.
We're currently not consistent with comma vs space, so either is fine with me
| ) | ||
| self.reduction_file_name_one_subfile = "TestCceReductions1.h5" | ||
| self.reduction_file_name_two_subfiles = "TestCceReductions2.h5" | ||
| plot_quantities = [ |
There was a problem hiding this comment.
(Optional) Is it possible to use the C++ routine that writes data in the simulation also to write data here? Would be nice to make sure the code keeps working.
There was a problem hiding this comment.
Not easily I don't think. This data is written from an iterable action that then calls either a threaded action or a local synchronous action to do the writing. The iterable action is pretty complex too
| @@ -100,6 +100,7 @@ def command(output, **kwargs): | |||
| ) | |||
| else: | |||
| plt.savefig(output) | |||
| plt.close() | |||
There was a problem hiding this comment.
For my information, did you encounter an issue with this?
There was a problem hiding this comment.
Yeah when I was running the unit test on my machine, even when I had the -o option specified, it would still open the plot interactively causing a timeout. This stopped that from happening.
Does not do any BMS transformations. Only plots raw CCE output
docs/Tutorials/CCE.md
Outdated
| r"Re $Y_{" + str(mode_l) + r"\," + str(mode_m) + r"}$", | ||
| r"Im $Y_{" + str(mode_l) + r"\," + str(mode_m) + r"}$" | ||
| ]) | ||
| If you'd like to do something more complex than just make a quick plot, you'll |
There was a problem hiding this comment.
maybe complex -> complicated? Since CCE deals with complex numbers, just to avoid any possible confusion? Up to you!
docs/Tutorials/CCE.md
Outdated
| \note The CLI can also plot the "old" version of CCE output, described above. | ||
| Pass `--cce-group Cce` to the CLI. This option is only for backwards | ||
| compatibility with the old CCE output and is not supported for the current | ||
| version of output. |
There was a problem hiding this comment.
Maybe add This option is deprecated and will be removed in the future.
|
@nilsdeppe Squashed in the changes |
Proposed changes
Also update the CCE tutorial to point to this CLI so we don't just have a large block of python code in our tutorial.
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments