search

fusion-energy / openmc-plasma-source

Creates a plasma source as an openmc.source object from input parameters that describe the plasma
MIT License
27 stars 11 forks source link

plotting functions should be methods of TokamakSource #47

Open RemDelaporteMathurin opened 2 years ago

RemDelaporteMathurin commented 2 years ago

I was looking at the functions in plotting and thought these should maybe be methods of TokamakSource so that users can do:


my_source = TokamakSouce(....)
my_source.plot_3d(...)
LiamPattinson commented 2 years ago

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...

shimwell commented 2 years ago

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

RemDelaporteMathurin commented 2 years ago

so that it can sample a source without making an initial_source.h5.

Hm... ops doesn't create initialèsource.h5 does it?

shimwell commented 2 years ago

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.

RemDelaporteMathurin commented 2 years ago

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!