From 05fe1dc675bae6e7473bb6c582ed1914d078c2dd Mon Sep 17 00:00:00 2001
From: benSepanski <ben_sepanski@alumni.baylor.edu>
Date: Mon, 29 Jun 2020 14:35:33 -0500
Subject: [PATCH] Improved docs

---
 doc/interop.rst                              | 60 +++++++++++++++-----
 meshmode/interop/firedrake/connection.py     | 53 ++++++++---------
 meshmode/interop/firedrake/mesh.py           | 55 +++++++++---------
 meshmode/interop/firedrake/reference_cell.py |  6 +-
 4 files changed, 104 insertions(+), 70 deletions(-)

diff --git a/doc/interop.rst b/doc/interop.rst
index 41ad3b63..e80fb8e8 100644
--- a/doc/interop.rst
+++ b/doc/interop.rst
@@ -4,36 +4,66 @@ interop
 Interfacing with data outside of :mod:`meshmode`.
 
 
-fiat
-----
-
-.. automodule:: meshmode.interop.fiat.simplex_cell
+Firedrake
+---------
 
+Function Spaces/Discretizations
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-FInAT
------
+.. automodule:: meshmode.interop.firedrake.connection
 
-.. automodule:: meshmode.interop.FInAT.lagrange_element
+Meshes
+^^^^^^
 
+.. automodule:: meshmode.interop.firedrake.mesh
 
-Firedrake
----------
+Reference Cells
+^^^^^^^^^^^^^^^
 
-TODO include this
+.. automodule:: meshmode.interop.firedrake.reference_cell
 
 
 Implementation Details
-----------------------
+^^^^^^^^^^^^^^^^^^^^^^
 
+Converting between :mod:`firedrake` and :mod:`meshmode` is in general
+straightforward. Some language is different:
 
-Firedrake Function Space Design
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+* In a mesh, a :mod:`meshmode` "element" is a :mod:`firedrake` "cell"
+* A :mod:`meshmode` :class:`Discretization` is a :mod:`firedrake`
+  :class:`WithGeometry` created, usually
+  created to using the function :func:`FunctionSpace` and referred
+  to as a "function space"
+* In a mesh, any vertices, faces, cells, etc. are :mod:`firedrake`
+  "entities" (see `dmplex <https://www.mcs.anl.gov/petsc/petsc-current/docs/manualpages/DMPLEX/index.html>`_
+  for more info on how topological mesh information is stored
+  in :mod:`firedrake`).
+
+Other than carefully tabulating how and which vertices/faces
+correspond to other vertices/faces/cells, there are two main difficulties.
+
+1. :mod:`meshmode` requires that all mesh elements be positively oriented,
+   :mod:`firedrake` does not.
+2. :mod:`meshmode` has discontinuous polynomial function spaces
+   which use different nodes than :mod:`firedrake`.
+
+Consequently, any :mod:`firedrake` :class:`firedrake.function.Function`
+whose data is converted onto a corresponding :class:`Discretization`
+using a :class:`FromFiredrakeConnection` instance is
+first reordered (as the converted mesh was reordered to have
+positively oriented elements) and then resampled at the :mod:`meshmode`
+nodes.
+
+For Developers: Firedrake Function Space Design Crash Course
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
 In firedrake, meshes and function spaces have a close relationship.
-In particular, due to some structure described in this
+In particular, this is  due to some structure described in this
 `firedrake pull request <http://github.com/firedrakeproject/firedrake/pull/627>`_.
-``fd2mm`` mimics this firedrake design style. 
+If you wish to develop on / add to the implementation of conversion
+between :mod:`meshmode` and :mod:`firedrake`, you will need
+to understand their design style. Below is a crash course.
 
 In short, it is the idea
 that every function space should have a mesh, and the coordinates of the mesh
diff --git a/meshmode/interop/firedrake/connection.py b/meshmode/interop/firedrake/connection.py
index 61977330..936e8732 100644
--- a/meshmode/interop/firedrake/connection.py
+++ b/meshmode/interop/firedrake/connection.py
@@ -22,6 +22,7 @@ THE SOFTWARE.
 
 __doc__ = """
 .. autoclass:: FromFiredrakeConnection
+    :members:
 """
 
 import numpy as np
@@ -43,14 +44,14 @@ from meshmode.discretization import Discretization
 
 def _reorder_nodes(orient, nodes, flip_matrix, unflip=False):
     """
-    flips :param:`nodes` in place according to :param:`orient`
+    flips *nodes* in place according to *orient*
 
-    :param orient: An array of shape *(nelements)* of orientations,
+    :arg orient: An array of shape *(nelements)* of orientations,
                  >0 for positive, <0 for negative
-    :param nodes: a *(nelements, nunit_nodes)* or shaped array of nodes
-    :param flip_matrix: The matrix used to flip each negatively-oriented
+    :arg nodes: a *(nelements, nunit_nodes)* or shaped array of nodes
+    :arg flip_matrix: The matrix used to flip each negatively-oriented
                       element
-    :param unflip: If *True*, use transpose of :param:`flip_matrix` to
+    :arg unflip: If *True*, use transpose of *flip_matrix* to
                  flip negatively-oriented elements
     """
     # reorder nodes (Code adapted from
@@ -88,8 +89,8 @@ class FromFiredrakeConnection:
     """
     def __init__(self, cl_ctx, fdrake_fspace):
         """
-        :param cl_ctx: A :mod:`pyopencl` computing context
-        :param fdrake_fspace: A :mod:`firedrake` ``"CG"`` or ``"DG"``
+        :arg cl_ctx: A :mod:`pyopencl` computing context
+        :arg fdrake_fspace: A :mod:`firedrake` ``"CG"`` or ``"DG"``
             function space (of class :class:`WithGeometry`) built on
             a mesh which is importable by :func:`import_firedrake_mesh`.
         """
@@ -97,14 +98,14 @@ class FromFiredrakeConnection:
         # element.
         from firedrake.functionspaceimpl import WithGeometry
         if not isinstance(fdrake_fspace, WithGeometry):
-            raise TypeError(":param:`fdrake_fspace` must be of firedrake type "
+            raise TypeError(":arg:`fdrake_fspace` must be of firedrake type "
                             ":class:`WithGeometry`, not `%s`."
                             % type(fdrake_fspace))
         ufl_elt = fdrake_fspace.ufl_element()
 
         if ufl_elt.family() not in ('Lagrange', 'Discontinuous Lagrange'):
             raise ValueError("the ``ufl_element().family()`` of "
-                             ":param:`fdrake_fspace` must "
+                             ":arg:`fdrake_fspace` must "
                              "be ``'Lagrange'`` or "
                              "``'Discontinuous Lagrange'``, not %s."
                              % ufl_elt.family())
@@ -180,7 +181,7 @@ class FromFiredrakeConnection:
         """
         Return a firedrake function space of the appropriate vector dimension
 
-        :param dim: Either *None*, in which case a function space which maps
+        :arg dim: Either *None*, in which case a function space which maps
                     to scalar values is returned, or a positive integer *n*,
                     in which case a function space which maps into *\\R^n*
                     is returned
@@ -211,12 +212,12 @@ class FromFiredrakeConnection:
         """
         transport firedrake function onto :attr:`to_discr`
 
-        :param function: A :mod:`firedrake` function to transfer onto
+        :arg function: A :mod:`firedrake` function to transfer onto
             :attr:`to_discr`. Its function space must have
             the same family, degree, and mesh as ``self.from_fspace()``.
-        :param out: If *None* then ignored, otherwise a numpy array of the
+        :arg out: If *None* then ignored, otherwise a numpy array of the
             shape *function.dat.data.shape.T* (i.e.
-            *(dim, nnodes)* or *(nnodes,)* in which :param:`function`'s
+            *(dim, nnodes)* or *(nnodes,)* in which *function*'s
             transported data is stored.
 
         :return: a numpy array holding the transported function
@@ -225,15 +226,15 @@ class FromFiredrakeConnection:
         # function space
         from firedrake.function import Function
         assert isinstance(function, Function), \
-            ":param:`function` must be a :mod:`firedrake` Function"
+            ":arg:`function` must be a :mod:`firedrake` Function"
         assert function.function_space().ufl_element().family() \
             == self._ufl_element.family() and \
             function.function_space().ufl_element().degree() \
             == self._ufl_element.degree(), \
-            ":param:`function` must live in a function space with the " \
+            ":arg:`function` must live in a function space with the " \
             "same family and degree as ``self.from_fspace()``"
         assert function.function_space().mesh() is self._mesh_geometry, \
-            ":param:`function` mesh must be the same mesh as used by " \
+            ":arg:`function` mesh must be the same mesh as used by " \
             "``self.from_fspace().mesh()``"
 
         # Get function data as shape [nnodes][dims] or [nnodes]
@@ -262,21 +263,21 @@ class FromFiredrakeConnection:
         transport meshmode field from :attr:`to_discr` into an
         appropriate firedrake function space.
 
-        :param mm_field: A numpy array of shape *(nnodes,)* or *(dim, nnodes)*
+        :arg mm_field: A numpy array of shape *(nnodes,)* or *(dim, nnodes)*
             representing a function on :attr:`to_distr`.
-        :param out: If *None* then ignored, otherwise a :mod:`firedrake`
+        :arg out: If *None* then ignored, otherwise a :mod:`firedrake`
             function of the right function space for the transported data
             to be stored in.
-        :param assert_fdrake_discontinuous: If *True*,
+        :arg assert_fdrake_discontinuous: If *True*,
             disallows conversion to a continuous firedrake function space
             (i.e. this function checks that ``self.from_fspace()`` is
              discontinuous and raises a *ValueError* otherwise)
-        :param continuity_tolerance: If converting to a continuous firedrake
+        :arg continuity_tolerance: If converting to a continuous firedrake
             function space (i.e. if ``self.from_fspace()`` is continuous),
             assert that at any two meshmode nodes corresponding to the
             same firedrake node (meshmode is a discontinuous space, so this
             situation will almost certainly happen), the function being transported
-            has values at most :param:`continuity_tolerance` distance
+            has values at most *continuity_tolerance* distance
             apart. If *None*, no checks are performed.
 
         :return: a :mod:`firedrake` :class:`Function` holding the transported
@@ -285,22 +286,22 @@ class FromFiredrakeConnection:
         if self._ufl_element.family() == 'Lagrange' \
                 and assert_fdrake_discontinuous:
             raise ValueError("Trying to convert to continuous function space "
-                             " with :param:`assert_fdrake_discontinuous` set "
+                             " with :arg:`assert_fdrake_discontinuous` set "
                              " to *True*")
         # make sure out is a firedrake function in an appropriate
         # function space
         if out is not None:
             from firedrake.function import Function
             assert isinstance(out, Function), \
-                ":param:`out` must be a :mod:`firedrake` Function or *None*"
+                ":arg:`out` must be a :mod:`firedrake` Function or *None*"
             assert out.function_space().ufl_element().family() \
                 == self._ufl_element.family() and \
                 out.function_space().ufl_element().degree() \
                 == self._ufl_element.degree(), \
-                ":param:`out` must live in a function space with the " \
+                ":arg:`out` must live in a function space with the " \
                 "same family and degree as ``self.from_fspace()``"
             assert out.function_space().mesh() is self._mesh_geometry, \
-                ":param:`out` mesh must be the same mesh as used by " \
+                ":arg:`out` mesh must be the same mesh as used by " \
                 "``self.from_fspace().mesh()`` or *None*"
         else:
             if len(mm_field.shape) == 1:
@@ -351,7 +352,7 @@ class FromFiredrakeConnection:
                     if dist >= continuity_tolerance:
                         raise ValueError("Meshmode nodes %s and %s represent "
                                          "the same firedrake node %s, but "
-                                         ":param:`mm_field`'s values are "
+                                         ":arg:`mm_field`'s values are "
                                          " %s > %s apart)"
                                          % (mm_inode, dup_mm_inode, fd_inode,
                                             dist, continuity_tolerance))
diff --git a/meshmode/interop/firedrake/mesh.py b/meshmode/interop/firedrake/mesh.py
index d9060b9b..a0c8cb0d 100644
--- a/meshmode/interop/firedrake/mesh.py
+++ b/meshmode/interop/firedrake/mesh.py
@@ -1,4 +1,4 @@
-__copyright__ = "Copyright (C) 2020 Benjamin Sepanski"
+arg = "Copyright (C) 2020 Benjamin Sepanski"
 
 __license__ = """
 Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -43,14 +43,14 @@ def _get_firedrake_nodal_info(fdrake_mesh_topology):
     are guaranteed to have the same numbering in :mod:`meshmode`
     as :mod:`firedrdake`
 
-    :param fdrake_mesh_topology: A :mod:`firedrake` instance of class
+    :arg fdrake_mesh_topology: A :mod:`firedrake` instance of class
         :class:`MeshTopology` or :class:`MeshGeometry`.
 
     :return: Returns *vertex_indices* as a numpy array of shape
         *(nelements, ref_element.nvertices)* (as described by
         the ``vertex_indices`` attribute of a :class:`MeshElementGroup`)
         and a :class:`NodalAdjacency` constructed from
-        :param:`fdrake_mesh_topology`
+        *fdrake_mesh_topology*
         as a tuple *(vertex_indices, nodal_adjacency)*.
     """
     top = fdrake_mesh_topology.topology
@@ -145,7 +145,7 @@ def _get_firedrake_boundary_tags(fdrake_mesh):
     any markers in the mesh topology's exterior facets
     (see :attr:`firedrake.mesh.MeshTopology.exterior_facets.unique_markers`)
 
-    :param fdrake_mesh: A :mod:`firedrake` :class:`MeshTopology` or
+    :arg fdrake_mesh: A :mod:`firedrake` :class:`MeshTopology` or
         :class:`MeshGeometry`
 
     :return: A tuple of boundary tags
@@ -167,7 +167,7 @@ def _get_firedrake_facial_adjacency_groups(fdrake_mesh_topology):
     have geometric information, elements may need to be
     flipped later.
 
-    :param fdrake_mesh_topology: A :mod:`firedrake` instance of class
+    :arg fdrake_mesh_topology: A :mod:`firedrake` instance of class
         :class:`MeshTopology` or :class:`MeshGeometry`.
     :return: A list of maps to :class:`FacialAdjacencyGroup`s as required
         by a :mod:`meshmode` :class:`Mesh`
@@ -266,13 +266,13 @@ def _get_firedrake_orientations(fdrake_mesh, unflipped_group, vertices,
     """
     Return the orientations of the mesh elements:
 
-    :param fdrake_mesh: A :mod:`firedrake` instance of :class:`MeshGeometry`
-    :param unflipped_group: A :class:`SimplexElementGroup` instance with
+    :arg fdrake_mesh: A :mod:`firedrake` instance of :class:`MeshGeometry`
+    :arg unflipped_group: A :class:`SimplexElementGroup` instance with
         (potentially) some negatively oriented elements.
-    :param vertices: The vertex coordinates as a numpy array of shape
+    :arg vertices: The vertex coordinates as a numpy array of shape
         *(ambient_dim, nvertices)*
-    :param normals: As described in the kwargs of :func:`import_firedrake_mesh`
-    :param no_normals_warn: As described in the kwargs of
+    :arg normals: As described in the kwargs of :func:`import_firedrake_mesh`
+    :arg no_normals_warn: As described in the kwargs of
         :func:`import_firedrake_mesh`
 
     :return: A numpy array, the *i*th element is > 0 if the *ith* element
@@ -325,7 +325,7 @@ def _get_firedrake_orientations(fdrake_mesh, unflipped_group, vertices,
 
 # {{{ Mesh conversion
 
-def import_firedrake_mesh(fdrake_mesh, **kwargs):
+def import_firedrake_mesh(fdrake_mesh, normals=None, no_normals_warn=None):
     """
     Create a :mod:`meshmode` :class:`Mesh` from a :mod:`firedrake`
     :class:`MeshGeometry` with the same cells/elements, vertices, nodes,
@@ -340,7 +340,7 @@ def import_firedrake_mesh(fdrake_mesh, **kwargs):
     The flipped cells/elements are identified by the returned
     *firedrake_orient* array
 
-    :param fdrake_mesh: A :mod:`firedrake` :class:`MeshGeometry`.
+    :arg fdrake_mesh: A :mod:`firedrake` :class:`MeshGeometry`.
         This mesh **must** be in a space of ambient dimension
         1, 2, or 3 and have co-dimension of 0 or 1.
         It must use a simplex as a reference element.
@@ -350,7 +350,7 @@ def import_firedrake_mesh(fdrake_mesh, **kwargs):
         have been called.
 
         In the case of a 1-dimensional mesh embedded in 2-space,
-        see the keyword arguments below.
+        see parameters *normals* and *no_normals_warn*.
 
         Finally, the ``coordinates`` attribute must have a function
         space whose *finat_element* associates a degree
@@ -358,21 +358,26 @@ def import_firedrake_mesh(fdrake_mesh, **kwargs):
         this means that the vertices of the mesh must have well-defined
         coordinates.
         For those unfamiliar with :mod:`firedrake`, you can
-        verify this by looking at the ``[0]`` entry of
-        ``fdrake_mesh.coordinates.function_space().finat_element.entity_dofs()``.
+        verify this by looking at
 
-    :Keyword Arguments:
-        * *normals*: _Only_ used if :param:`fdrake_mesh` is a 1-surface
-          embedded in 2-space. In this case,
+        .. code-block:: python
+
+            import six
+            coords_fspace = fdrake_mesh.coordinates.function_space()
+            vertex_entity_dofs = coords_fspace.finat_element.entity_dofs()[0]
+            for entity, dof_list in six.iteritems(vertex_entity_dofs):
+                assert len(dof_list) > 0
+    :arg normals: **Only** used if *fdrake_mesh* is a 1-surface
+        embedded in 2-space. In this case,
             - If *None* then
               all elements are assumed to be positively oriented.
             - Else, should be a list/array whose *i*th entry
               is the normal for the *i*th element (*i*th
-              in :param:`mesh`*.coordinate.function_space()*'s
-              :attribute:`cell_node_list`)
-        * *no_normals_warn: If *True* (the default), raises a warning
-          if :param:`fdrake_mesh` is a 1-surface embedded in 2-space
-          and :param:`normals` is *None*.
+              in *mesh.coordinate.function_space()*'s
+              :attr:`cell_node_list`)
+    :arg no_normals_warn: If *True* (the default), raises a warning
+        if *fdrake_mesh* is a 1-surface embedded in 2-space
+        and *normals* is *None*.
 
     :return: A tuple *(meshmode mesh, firedrake_orient)*.
          ``firedrake_orient < 0`` is *True* for any negatively
@@ -383,7 +388,7 @@ def import_firedrake_mesh(fdrake_mesh, **kwargs):
     # Type validation
     from firedrake.mesh import MeshGeometry
     if not isinstance(fdrake_mesh, MeshGeometry):
-        raise TypeError(":param:`fdrake_mesh_topology` must be a "
+        raise TypeError(":arg:`fdrake_mesh_topology` must be a "
                         ":mod:`firedrake` :class:`MeshGeometry`, "
                         "not %s." % type(fdrake_mesh))
     assert fdrake_mesh.ufl_cell().is_simplex(), "Mesh must use simplex cells"
@@ -457,8 +462,6 @@ def import_firedrake_mesh(fdrake_mesh, **kwargs):
     vertices = np.array([vertices[i] for i in range(len(vertices))]).T
 
     # Use the vertices to compute the orientations and flip the group
-    normals = kwargs.get('normals', None)
-    no_normals_warn = kwargs.get('no_normals_warn', True)
     orient = _get_firedrake_orientations(fdrake_mesh, unflipped_group, vertices,
                                          normals=normals,
                                          no_normals_warn=no_normals_warn)
diff --git a/meshmode/interop/firedrake/reference_cell.py b/meshmode/interop/firedrake/reference_cell.py
index b8509582..153c5667 100644
--- a/meshmode/interop/firedrake/reference_cell.py
+++ b/meshmode/interop/firedrake/reference_cell.py
@@ -38,8 +38,8 @@ def get_affine_reference_simplex_mapping(spat_dim, firedrake_to_meshmode=True):
     on one reference cell and maps each
     point to another using a positive affine map.
 
-    :param spat_dim: The spatial dimension
-    :param firedrake_to_meshmode: If true, the returned function maps from
+    :arg spat_dim: The spatial dimension
+    :arg firedrake_to_meshmode: If true, the returned function maps from
         the firedrake reference element to
         meshmode, if false maps from
         meshmode to firedrake. More specifically,
@@ -114,7 +114,7 @@ def get_finat_element_unit_nodes(finat_element):
     Returns the unit nodes used by the FInAT element in firedrake's
     (equivalently, FInAT/FIAT's) reference coordinates
 
-    :param finat_element: A :class:`finat.finiteelementbase.FiniteElementBase`
+    :arg finat_element: A :class:`finat.finiteelementbase.FiniteElementBase`
         instance (i.e. a firedrake function space's reference element).
         The refernce element of the finat element *MUST* be a simplex
     :return: A numpy array of shape *(dim, nunit_nodes)* holding the unit
-- 
GitLab