-
Notifications
You must be signed in to change notification settings - Fork 11
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
plotting functions should be methods of TokamakSource #47
Comments
I tend to be of the opinion that objects shouldn't know how to plot themselves. The user may have no intention of plotting TokamakSource, but making those methods part of the class means they now must take on another non-optional dependency. It can also cause class bloat, and as the project grows the amount of responsibilities held by one class can get out of hand. Then again, the syntax is quite nice. Perhaps something like this would work? def plot_3D(self,*args,**kwargs):
from .plotting import plot_tokamak_source_3D
plot_tokamak_source_3D(self,*args,**kwargs) It permits the use of either method, and keeps class bloat to a minimum, though I'm generally not a fan of importing things anywhere except the top of a file. I think it might be considered a PEP8 issue, and it can lead to frustrating errors. However, it does work... |
Once the openmc.lib has access to the source sampling the I plan to update this package so that it can sample a source without making an initial_source.h5. This is a longer term plan but I hope to make openmc_source_plotter into a nice source plotting package for any openmc source |
Hm... ops doesn't create initialèsource.h5 does it? |
ops (openmc-plasma-source) has a plotting function for the tokamak source but it doesn't make use of the openmc distributions to plot. It doesn't actually plot the source as openmc sees it. Therefore ops doesn't create an initial_source.h5 as it doesn't make use of openmc to sample energy, position or direction. ops doesn't plot the actual positions, energies or directions of neutrons that are created in the neutronics code. So unless we want to program in every distribution that OpenMC understands I think we should make use of OpenMC to sources sampling routines to get the source details. This inital_source feature is only in version 0.11 for fixed sources and unfortunate version 0.11 doesn't include the ring source. So that is why I suggest we wait for openmc.lib to support source sampling. |
Gotcha sorry I thought you meant that currently ops ued initial_source.h5 but I must've gotten mixed up. It would be so much better if we could also plot the source as openmc sees it! |
I was looking at the functions in plotting and thought these should maybe be methods of TokamakSource so that users can do:
The text was updated successfully, but these errors were encountered: