-
Notifications
You must be signed in to change notification settings - Fork 1
Added capability to StageWakeT to save simulation outputs along the stage and some bugfixes
#158
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: main
Are you sure you want to change the base?
Conversation
…_wake_t to make it always dump one time step.
…iles along the stage. Also corrected docstrings.
…ction signature of StageWakeT.plot_waterfalls() and edited docstrings.
… StageWakeT to remove unnecessary parameters.
| keep_data : bool | ||
| Flag for whether to keep raw Wake-T output after simulation. | ||
| output_period : int, optional | ||
| Number of times along the stage in which the Wake-T diagnostics data |
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.
Should be "output every output_period time steps", also point to wake_t.Plasma_stage n_out parameter?
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.
Should this be a standard parameter for ALL Stage classes?
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.
How does this interact with initial and final step outputs?
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.
Unit of "dz" does not work well, since it is unknown to the user. A better idea would be to set the period in meters, or to set the number of outputs (as in n_out for wake_t)
| wakefield_model='quasistatic_2d' | ||
|
|
||
| if self.output_period is None: | ||
| self.output_period = round(self.length/dz/8) |
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.
Add comment: "Defaults to 8 times throughout the stage"
| plasma = wake_t.PlasmaStage(length=self.length, density=plasma_profile, wakefield_model=wakefield_model, | ||
| r_max=box_size_r, r_max_plasma=box_size_r, xi_min=box_min_z, xi_max=box_max_z, | ||
| n_out=n_out, n_r=int(self.num_cell_xy), n_xi=int(num_cell_z), dz_fields=dz, ppc=1) | ||
| n_out=self.output_period, n_r=int(self.num_cell_xy), n_xi=int(num_cell_z), dz_fields=dz, ppc=1) |
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.
Is this correct?
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.
n_out has a different meaning...
kyrsjo
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.
Confusion about meaning. Propose to move this parameter to Stage class, and define clearly, i.e. as the number of outputs (with clarification if this should include final/initial steps), or period in meters for "in between" outputs.
Bugfixes involve class methods generating waterfall plots not importing the correct functions and waterfall plots not being saved.