ansys / pymapdl

Pythonic interface to MAPDL
https://mapdl.docs.pyansys.com
MIT License
421 stars 119 forks source link

Use theme instead of several optional parameters for general_plotter #425

Open natter1 opened 3 years ago

natter1 commented 3 years ago

Edit: Progress

It might be useful, to make second and third part into new issues.

EndEdit

Sorry for spamming issues, but as I play with the plot functionality (including pyvista, which is also new for me), I get a lot of ideas ^^ There are many parameters to change the look of plots, and also some missing parameters (e.g. to change label format). Even worse, by setting some of these to default values, its not possible to use pyvista themes like 'document' or 'night' without "removing" those default values (by setting parameters to ''). Lables can't be changed at all.

def plot_geometry(mapdl: MapdlCorba, theme="document"):
    pv.set_plot_theme(theme)
    plotter = pv.Plotter(shape=(2, 2))
    mapdl.nplot(plotter=plotter, color='')
    plotter.subplot(0, 1)
    mapdl.lplot(plotter=plotter, color='')#color_lines=True) # color="#000000")
    plotter.subplot(1, 0)
    mapdl.aplot(plotter=plotter, color='')
    plotter.subplot(1, 1)
    mapdl.eplot(plotter=plotter, color='')
    plotter.link_views()
    plotter.show(screenshot=True)
    plotter.screenshot(f'multiplot_example')

multiplot_example

I think it would be better to use a Theme to format most of the plot. It would be much cleaner and easier to use. Up till now I didn't find, how to use/add a custom theme with pyvista. But it seems like a simple dict to me - so it should be easy to add for pyvista, if it is really not implemented yet.

akaszynski commented 3 years ago

Sorry for spamming issues, but as I play with the plot functionality

No issue here. I appreciate the inputs because it's helping drive the direction. I can't guarantee I'll get back to all these issues quickly due to the volume of demands.

Up till now I didn't find, how to use/add a custom theme with pyvista. But it seems like a simple dict to me - so it should be easy to add for pyvista, if it is really not implemented yet.

Theme can be set with https://docs.pyvista.org/examples/02-plot/themes.html

I think it would be better to use a Theme to format most of the plot

Quite true. We do this a little bit by default with: https://github.com/pyansys/pymapdl/blob/36598e484987308374e742833a31309243b010a4/ansys/mapdl/core/misc.py#L105-L112

Which is called on init of pymapdl. Changes need to be made module-wide to ensure that we're not overriding the theme with each method.

natter1 commented 3 years ago

Theme can be set with https://docs.pyvista.org/examples/02-plot/themes.html

I found that one. But I couldn't find anything to set a user defined theme. And I'm also not sure, which way would be the best to do it (at least I guess for now it doesn't matter whether its implemented in pyvista or pymapdl). Basicaly I see two options (there might be more):

Changes need to be made module-wide to ensure that we're not overriding the theme with each method

Thats also something I thought about. Its quite tricky. I see two different approaches here too.

If themes are implemented as a class, it might be worth to consider implementing a functionallity, which saves all the original rcParams-values before setting new ones and add a method restore_original()

Another alternative would be some kind of stylesheet-class, which only formats a given plot (without touching rcParams). That could be given as a parameter to a plot-function. If None, the plot-functions would fall back to the theme.

akaszynski commented 3 years ago
  • Create a class Theme, which provides an easy to understand interface for the user. The class could set the theme by itself or give a correct dict to pv.set_plot_theme(). A class could come with doc-strings, which is also a plus. When something has to change I would expect its easier to catch errors in legacy code compared to a pure dict, where some keys or values have become invalid.

Theme class seems preferable, something like:

class Theme():
    """Control the default plotting theme for ``pyvista``"""

    def __init__():
        self._background = [0.3, 0.3, 0.3]

    @property
    def background(self):
        """Default background color of a pyvista plot.

        Examples
        --------
        Set the default global background of all plots to white.

        >>> pyvista.theme.background = 'white'
        """
        return self._background

    @background.setter
    def background(self, new_background):
        self._background = parse_color(new_background)

This is preferable as we can incorporate some basic type checking when setting these values, rather than from a dictionary where the properties are not apparent in an IDE via code completion.

natter1 commented 3 years ago

@akaszynski I guess it will take a while until the new Theme-class becomes availeable in a new pyvista release, and afterwards in a new release of pymapdl. If you don't plan to implement it right away in pymapdl (I can't match your speed ^^), I would start to gather the different places where pymapdl is setting plot-format in a plotting_themes.py using the new pyvista themes over the weekend. Might help when deciding how to restructure pymapdl to use themes.

akaszynski commented 3 years ago

@akaszynski I guess it will take a while until the new Theme-class becomes availeable in a new pyvista release, and afterwards in a new release of pymapdl. If you don't plan to implement it right away in pymapdl (I can't match your speed ^^), I would start to gather the different places where pymapdl is setting plot-format in a plotting_themes.py using the new pyvista themes over the weekend. Might help when deciding how to restructure pymapdl to use themes.

I think this feature warrants a quick release as it's a critical feature for modules that rely on pyvista. I'll try to push one out soon, potentially over the weekend.

natter1 commented 3 years ago

I have looked into the default APDL plotting functions. Here is a summary plotting_themes.py

The short version:

class GeneralPlotterStyle(EmptyStyle):
    def __init__(self):
        super().__init__()
        self.title = ''
        self.background = None
        self.window_size = None
        self.notebook = None
        # add_mesh kwargs:
        self.color = 'w'
        self.show_edges = None
        self.edge_color = None
        self.lighting = None
        self.cmap = None
        self.render_points_as_spheres = False
        self.smooth_shading = None

        # parameters not part of pyvista.DefaultTheme()
        self.cpos = None
        self.show_bounds = False
        self.show_axes = True
        self.off_screen = None
        self.savfig = None
        self.point_size = 5.0
        self.line_width = None
        self.opacity = 1.0
        self.flip_scalars = False
        self.n_colors = 256
        self.interpolate_before_map = True
        self.render_lines_as_tubes = False
        self.stitle = None  # todo: stitle is deprecated in pyvista -> scalar_bar_args

        # todo: up till now this is to format labels - not the same as pyvista!
        self.font.size = None
        self.font.family = None
        self.text_color = None

class APlotStyle(EmptyStyle):
    # Type Hints
    vtk: Optional[bool]
    quality: Literal[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
    show_area_numbering: bool
    show_line_numbering: bool
    color_areas: bool
    show_lines: bool
    stitle: Optional[str]

    def __init__(self):
        super().__init__()
        self.title = "MAPDL Area Plot"

        # parameters not part of pyvista.DefaultTheme()
        self.vtk = None  # todo: what does None mean?
        self.quality = 4
        self.show_area_numbering = False
        self.show_line_numbering = False
        self.color_areas = False
        self.show_lines = False
        self.stitle = None  # -> where to put instead?

class EPlotStyle(EmptyStyle):
    show_node_numbering: bool
    vtk: Optional[bool]

    def __init__(self):
        super().__init__()
        self.title = 'MAPDL Element Plot'
        self.show_edges = True

        # parameters not part of pyvista.DefaultTheme()
        self.show_node_numbering = False
        self. vtk = None  # -> vtk = self._use_vtk

class KPlotStyle(EmptyStyle):
    vtk: Optional[bool]
    show_keypoint_numbering: bool

    def __init__(self):
        super().__init__()
        self.title = 'MAPDL Keypoint Plot'

        # parameters not part of pyvista.DefaultTheme()
        self.vtk = None  # -> vtk = self._use_vtk
        self.show_keypoint_numbering = True

class LPLOTStyle(EmptyStyle):
    vtk: Optional[bool]
    show_line_numbering: bool
    show_keypoint_numbering: bool
    color_lines: bool

    def __init__(self):
        super().__init__()
        self.title = 'MAPDL Line Plot'

        # parameters not part of pyvista.DefaultTheme()
        self.vtk = None  # -> vtk = self._use_vtk
        self.show_line_numbering = True
        self.show_keypoint_numbering = False
        self.color_lines = False

class NPLOTStyle(EmptyStyle):
    vtk: Optional[bool]

    def __init__(self):
        super().__init__()
        self.title = 'MAPDL Node Plot'

        # parameters not part of pyvista.DefaultTheme()
        self.vtk = None  # -> vtk = self._use_vtk

class VPLOTStyle(EmptyStyle):
    vtk: Optional[bool]
    quality: Literal[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
    show_area_numbering: bool
    show_line_numbering: bool
    color_areas: bool
    show_lines: bool

    def __init__(self):
        super().__init__()
        # parameters not part of pyvista.DefaultTheme()
        self.vtk = None  # -> vtk = self._use_vtk
        self.quality = 4
        self.show_area_numbering = False
        self.show_line_numbering = False
        self.color_areas = False
        self.show_lines = True

There are a lot of pymapddl-specific parameters. The question here is, if we add this to a PyMAPDLTheme or add an addittional class-object or leave it as function/method-parameters.

natter1 commented 3 years ago

In post.py we have a lot of similar plot_nodal... methods. Theoretically we could replace them with a single method plot(solution: NodalSolution), where NodalSolution is an Enum-class. That would make testing, changing the interface and updating doc-strings a lot easier. I'm not so sure about the user-perspective. It would be neccessary to import and use an additional Enum.

natter1 commented 3 years ago
class NodalSolution(Enum):
    Displacement_X = auto()
    Displacement_Y = auto()
    Displacement_Z = auto()
    Displacement_NORM = auto()

    Rotation_X = auto()
    Rotation_Y = auto()
    Rotation_Z = auto()

    Temperature = auto()
    # ...

class _PostProcessing(PostProcessing):
    def __init__(self, mapdl):
        # todo: move to PostProcessing (allows function as self.nodal_displacement ...)
        super().__init__(mapdl)
        self._plot_dict = {  # scalar function, stitle, comp
            NodalSolution.Displacement_X: (self.nodal_displacement, 'X Displacement', 'X'),
            NodalSolution.Displacement_Y: (self.nodal_displacement, 'Y Displacement', 'Y'),
            NodalSolution.Displacement_Z: (self.nodal_displacement, 'Z Displacement', 'Z'),
            NodalSolution.Displacement_NORM: (self.nodal_displacement, 'NORM Displacement', 'NORM'),
            NodalSolution.Rotation_X: (self.nodal_rotation, 'X Rotation', 'X'),
            NodalSolution.Rotation_Y: (self.nodal_rotation, 'Y Rotation', 'y'),
            NodalSolution.Rotation_Z: (self.nodal_rotation, 'Z Rotation', 'Z'),
            NodalSolution.Temperature: (self.nodal_temperature, 'Nodal Temperature', None),
            # ...
        }

    def plot(self, solution: NodalSolution, show_node_numbering=False, **kwargs):
        func, _stitle, comp = self._plot_dict[solution]
        kwargs.setdefault('stitle', _stitle)
        if comp is not None:
            scalars = func(comp)
        else:
            scalars = func()

        return self._plot_point_scalars(scalars, show_node_numbering=show_node_numbering,
                                        **kwargs)

Even if we do not implement it this way, it might be a good testing solution without messing with the old interface. If this wotks as expected, its easy to transfer to the plot_nodal_...() methods.

akaszynski commented 3 years ago

In post.py we have a lot of similar plot_nodal... methods. Theoretically we could replace them with a single method plot(solution: NodalSolution), where NodalSolution is an Enum-class. That would make testing, changing the interface and updating doc-strings a lot easier. I'm not so sure about the user-perspective. It would be neccessary to import and use an additional Enum.

That needs to be implemented. I'm going to make a hefty PR to patch plotting along with using the theme Class just merged with pyvista. I'll do some basic testing and push a patch for 0.30.2 tomorrow.

jgd10 commented 3 years ago

Can this issue be closed? It looks like it might have been resolved?

akaszynski commented 3 years ago

Can this issue be closed? It looks like it might have been resolved?

This needs https://github.com/pyvista/pyvista/pull/1359 then a new PR that implements a theme.

natter1 commented 3 years ago

PR #477

akaszynski commented 3 years ago

discuss/add additional values for MapdlTheme class (numbering, etc.)

What additional kwargs/attributes do we need to add to MapdlTheme?

natter1 commented 3 years ago

What additional kwargs/attributes do we need to add to MapdlTheme?

I'm not sure if we need it (thats up for discussion imho). In the summary about plotting functions, I found many parameters that are not part of pyvista Theme, and might be added to MapdlTheme (or maybe not ^^).

        # general_plotter()
        # parameters not part of pyvista.DefaultTheme()
        self.cpos = None
        self.show_bounds = False
        self.show_axes = True
        self.off_screen = None
        self.savfig = None
        self.point_size = 5.0
        self.line_width = None
        self.opacity = 1.0
        self.flip_scalars = False
        self.n_colors = 256
        self.interpolate_before_map = True
        self.render_lines_as_tubes = False

        # todo: up till now this is to format labels - not the same as pyvista!
        self.font.size = None
        self.font.family = None
        self.text_color = None
        # aplot()
        # parameters not part of pyvista.DefaultTheme()
        self.vtk = None  # todo: what does None mean?
        self.quality = 4
        self.show_area_numbering = False
        self.show_line_numbering = False
        self.color_areas = False
        self.show_lines = False

Interesting might be e.g. show_bounds, show_axes, point_size, line_width, opacity, n_colors, render_lines_as_tubes, quality, show_line_numbering, show_area_numbering. Also, what about the label-format (e.g. size, color, backgroundcolor)?

germa89 commented 2 years ago

This probably goes in line with making plotting an external library where the plotting is made by a class.

Or maybe we just need to call pyvista........

germa89 commented 1 year ago

In #1938 we allow to pass more arguments to Pyvista object. It should alleviate the problem of customising plots.