RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.27k stars 1.26k forks source link

implement system diagrams for pydrake documentation #12590

Closed RussTedrake closed 3 years ago

RussTedrake commented 4 years ago

The nice html diagrams that we have for doxygen result in a totally scare mess for pydrake users.
For example: https://drake.mit.edu/doxygen_cxx/classdrake_1_1multibody_1_1_multibody_plant.html vs https://drake.mit.edu/pydrake/pydrake.multibody.plant.html#pydrake.multibody.plant.MultibodyPlant_

The doxygen code for this is just a very simple set of (recursive) aliases here: https://github.com/RobotLocomotion/drake/blob/master/doc/Doxyfile_CXX.in#L43

Presumably we could write a simple script that does this work somewhere around generate_pybind_documentation_header?

Also, note that we occasionally use html tags in our doxygen (including the multibodyplant example above) which, left unrendered, is not only messy but makes it impossible to parse ideas like "the green output ports do X". Is there a better way for sphinx to handle those html tags?

Note: we regularly receive feedback (e.g. from students) that the pydrake documentation needs a lot of work. I'll try to open some issues for ideas of possible "low hanging fruit" that might go a long way to improving that experience.

RussTedrake commented 4 years ago

I've got this working from C++ code via a doxygen INPUT_FILTER and an edit to mkdoc.py. (PR coming shortly)

But we still need to implement a custom sphinx directive for systems written in Python

cc @jpsaenz

RussTedrake commented 4 years ago

I've written the simple javascript version of the system yaml to html, and spent a little time this morning trying to integrate it into the drake sphinx autodoc as a directive. But wow. pydrake_sphinx_extension.py is almost impenetrable. I'm afraid I'm going to need help, @EricCousineau-TRI and/or @jamiesnape, to make this land.

I've put a demo of the javascript here: https://observablehq.com/@russtedrake/drake-system-yaml-to-html

I started by adding a file bindings/pydrake/doc/_static/system_yaml.js containing

function system_yaml(sys) {
  let input_port_html = "";
  if ('input_ports' in sys) {
    sys.input_ports.forEach(port => {
      input_port_html += `<tr><td align=right style=\"padding:5px 0px 5px 0px\">${port}&rarr;</td></tr>`;
    });
  }
  let output_port_html = "";
  if ('output_ports' in sys) {
    sys.output_ports.forEach(port => {
      output_port_html += `<tr><td align=left style=\"padding:5px 0px 5px 0px\">&rarr; ${port}</td></tr>`;
    });
  }
  return html`<table align=center cellpadding=0 cellspacing=0><tr align=center><td style=\"vertical-align:middle\"><table cellspacing=0 cellpadding=0>${input_port_html}</table></td><td align=center style=\"border:solid;padding-left:20px;padding-right:20px;vertical-align:middle\" bgcolor=#F0F0F0>${sys.name}</td><td style=\"vertical-align:middle\"><table cellspacing=0 cellpadding=0>${output_port_html}</table></td></tr></table>`;
}

function system_yaml_from_str(str) {
  //  let sys = yaml.safeLoad(str);
  let sys = {
    name: 'ManipulationStation',
    input_ports: ['a', 'b', 'c'],
    output_ports: ['d', 'e', 'f']
  };

  return system_yaml(sys);  
}

Calling app.add_js_file("system_yaml.js") in pydrake_sphinx_extension.py seemed to work and felt like the right first step. What remains is, I think, to add an autodoc directive (potentially only to the ClassLevelDocumenter) that would allow us to slightly change lines like this, into e.g.

... system_yaml::
        name: PointCloudConcatenation
        input_ports:
        - point_cloud_CiSi_id0
        - X_FCi_id0
        - ...
        - point_cloud_CiSi_idN
        - X_FCi_idN
        output_ports:
        - point_cloud_FS
EricCousineau-TRI commented 4 years ago

Yeah, I think going the Sphinx-proper route, and writing a custom reStructuredText directive parser + HTML emitter will be best.

Also, I'm not entirely sure if we need a JavaScript implementation if all it does is emit static HTML at this point? We can update the emitter once we have a better solution for #13874?

Or I could see us keeping the JavaScript iff it's used in Doxygen as well, and then we get diagrams in both for free? (and then can hot-swap other solutions?)

RussTedrake commented 4 years ago

We already have a python implementation of this in doc/system_doxygen.py

I would like a javascript version of this to exist somewhere so i can include systems easily in my course notes/ slides. Maybe there is a better place for it to live (that would be hosted on html)?

On Sat, Sep 5, 2020 at 1:17 PM Eric Cousineau notifications@github.com wrote:

Yeah, I think going the Sphinx-proper route, and writing a custom reStructuredText directive parser + HTML emitter will be best.

Also, I'm not entirely sure if we need a JavaScript implementation if all it does is emit static HTML at this point?

We can update the emitter once we have a better solution for #13874 https://github.com/RobotLocomotion/drake/issues/13874?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/RobotLocomotion/drake/issues/12590#issuecomment-687638054, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRE2NHNAUYYQNVVA23WTLLSEJXCXANCNFSM4KH3WOBQ .

EricCousineau-TRI commented 4 years ago

We already have a python implementation of this in doc/system_doxygen.py

Gotcha. Yeah, that could be used the by sphinx extension, then. We should just move the comment-reformatting logic outside of that function.

I would like a javascript version of this to exist somewhere [...]

Not sure what you really mean by this. Can you scribe out an explicit workflow?

Like, you call pydrake_wrapped_help(my_system) in a Jupyter notebook, and it renders HTML for you, rather than the Sphinx directive? If that's the case, this may be a bit finicky, but there are probably solutions for partially rendering reStructuredText to HTML in Jupyter sessions.

RussTedrake commented 4 years ago

If you look at the drake diagrams in my course notes right now, I’m copying over the html tables. That is clunky, and harder to maintain. I’d prefer to include eg https://drake.mit.edu/somepath/system_html.js in my header then just call system_html with my yaml instead.

RussTedrake commented 4 years ago

If we think i’m the only likely consumer, then i’ll just put the js file in my course notes for now.

EricCousineau-TRI commented 4 years ago

Gotcha. So not in Python at all; instead, you want to consolidate web (html) representations of systems in JavaScript, and then provide public API to that JavaScript, and incur the possible costs of backwards compatibility etc?

It'd be doable, but really, it'd be best if you just re-vendored that JavaScript code in underactuated / manipulation. (Er, typed this out, and you just stated as much).

That being said, I think you're expanding the scope of this issue from "render nice stuff in pydrake docs" to "consolidate system diagram rendering and share as JavaScript API"?

RussTedrake commented 4 years ago

Let's stick with "render nice stuff in pydrake docs". I merely thought js would be useful in that regard. But you're right that sphinx can just use a very slightly modified version of the python method.

EricCousineau-TRI commented 4 years ago

Queuing up for next 2 weeks.

RussTedrake commented 3 years ago

@EricCousineau-TRI ping. can i ask you to knock this one out soon?

EricCousineau-TRI commented 3 years ago

Sure.

EricCousineau-TRI commented 3 years ago

In review, confirmed that you did most of the work in #13642, saw that it just needs the Sphinx directive.

I'm doing that now, with a minor change - I will change the behavior to spit out:

.. pydrake_system::
    {yaml}

rather than the more raw form:

.. raw:: html
    {converted_to_html}

Should then ensure that same code paths are used for both C++-translated docs and pure Python-authored docs.

RussTedrake commented 3 years ago

This should not have been closed, I think? @EricCousineau-TRI did #14500, but for example, https://drake.mit.edu/pydrake/pydrake.manipulation.simple_ui.html?highlight=jointsliders#pydrake.manipulation.simple_ui.JointSliders still renders as image

when the intention was for the yaml string to turn into one my system diagrams?

EricCousineau-TRI commented 3 years ago

Sorry, was unclear if responsibility was to percolate those changes to all call sites. Easy enough to do.

For example, simple_ui just has a "malformed" docstring: https://github.com/RobotLocomotion/drake/blob/661f257f3e63b3e1e58843dacc4e519a81c7c9d4/bindings/pydrake/manipulation/simple_ui.py#L24-L27 should really be:

.. pydrake_system::

    name: JointSliders 
    output_ports: 
    - positions