Improve documentation (ongoing)

[brosaplanella]

There are quite a few features that are not in the docs, and which would be fairly minor fixes. Opening a list here so we can keep track of them as we go and fix them in batches. Feel free to add more in the comments, and we can update the list.

• [x] print_parameter_info not documented in base_model.py (#3584)
• [ ] Experiment steps docstrings do not mention that arrays can be passed as values for drive cycles.

User Guide

• [ ] Submodel docs: At the moment the documentation for the base models in pyamm just points to the relevent papers, but this is not really sufficient given that users can customise each model via submodels. As a proposal, I think there should be hierarchical user documentation on each of the models. At the top level there are the three main default models (SPM, SPMe, DFN), with a description of the relevent equations that is divided up using the submodel structure, so that a user can see the list of equations that are in each submodel, along with the parameters in that submodel (at the moment, its very diffiicult to know what parameters are needed for what submodels). Below this there should be an individual discussion of each submodel and the different options for each one. For each option the corresponding equations and parameters should be provided. Incompatible submodels should be highlighted. (edit by @martinjrobins)

• [ ] Getting started tutorials: The getting started tutorials should have an expanded section on submodels, how to vary them, and how to determine what parameters are needed by your particular model configuration. (@martinjrobins)

• [ ] Getting started tutorials: These docs introduce a lot of user-facing pybamm classes like Simulation, and should link to the API docs for these classes so that users can see the full range of options for these classes/functions. (edit by @martinjrobins)

• [x] #4532

• [ ] The user-guide, or main page, needs a "How to get help" section explictly pointing to discussions, issues, slack channel and explaining how to use each (@martinjrobins)

• [x] In the install from source we should make clearer that you do not need to create a virtual environment if using nox, as nox will create one for you.

API Docs

• [ ] The overall structure of the classes is not documented or explained. Needs to be a diagram, plus a lot of explanation on the class structure and dependencies (@martinjrobins )
• [x] #4535
• [ ] Generally make clear where classes implement getitem and/or setitem and so these are the expected calling patterns. (from @ejfdickinson )
• [ ] pybamm.Symbol. This is only a one-liner docstring, needs to explain the overall expression tree structure, with links to example notebooks, and include some examples of use, particularly explaining how to use all the overloaded operators (@martinjrobins )
• [ ] pybamm.Symbol and all inherited classes: domain, domains and auxillary_domains are confusing and we should resolve this asap (i.e. deprecate and remove domains and auxillary_domains (@martinjrobins)
• [ ] pybamm.Variable. domain arg needs to be documented (or removed entirely in favor of domains) (@martinjrobins)
• [ ] pybamm.BaseBatteryModel. Only a one-liner docstring, needs significant expansion. Explain the structure and purpose of the class, and examples of use. Link to example notebooks for more examples (@martinjrobins)
• [ ] pybamm.BaseBatteryModel. The info here (mainly in pybamm.BatteryModelOptions) is essential, user-facing info on the submodel configuration options, so should be much more visible (e.g. much of it should be copied over to a section in the user guide). There is no information on which submodel configurations conflict with each other, and only some of the options indicate what equations are added/removed, or what parameters are added/removed. I feel like each option should be its own page / example notebook, linking to publications where relevent and there should be much more detailed information at the equation/parameter level so users know exactly what model they are constructing (@martinjrobins )

API Docs - Small fixes suitable for first time contributors

• [ ] make explicit that pybamm.Solution implements a dict of pybamm.ProcessedVariable instances, so that the standard calling pattern is using getitem() on the pybamm.Solution and then using a method of the pybamm.ProcessedVariable class. (from @ejfdickinson )
• [ ] pybamm.ProcessedVariable documents the public attribute data as "the same as entries" but doesn't document entries. (from @ejfdickinson )
• [x] pybamm.Simulation.save, filename arg is not documented (https://github.com/pybamm-team/PyBaMM/pull/3602)
• [ ] pybamm.Simulation.set_parameters, pybamm.Simulation.set_up_and_parameterise_experiment and pybamm.Simulation.set_up_and_parameterise_model_for_experiment should be private (i.e. begin with underscore _) and not documented (tracked by #3751)
• [ ] pybamm.Experiment. period arg type is not linked, perhaps should be str rather than string. Also this needs to list possible string values this can take
• [x] #3724
• [ ] pybamm.BaseSolver.solve. Arg t_eval can be None, a list of numeric values or a numpy ndarray. Arg calc_sensitivities is optional
• [ ] pybamm.BaseSolver.step. Arg `save is optional
• [ ] pybamm.ScipySolver. Link to scipy.integrate.solve_ivp and format as code
• [ ] pybamm.JaxSolver. Arg method is optional. Link to jax.experimental.odeint and format as code. For method=‘BDF’ option summarise method used (use docstrings in jax_bdf_integrate.py) or simply link to the docstrings in jax_bdf_integrate.py
• [ ] pybamm.IDAKLUSolver. Arg root_method: should link to algebraic solver class and scipy.optimize.root (format the latter as code).
• [ ] pybamm.IDAKLUSolver. Arg extrap_tol defaults to None, not zero. Need to explain what None means, and what extrapolation is
• [ ] pybamm.QuickPlot. Arg labels should be same length as solutions, raises a ValueError if not. There are various bits of code in this docstring, which should be formated as inline code
• [ ] pybamm.QuickPlot.create_gif. remove round brackets around optional keyword (#3754)
• [ ] pybamm.QuickPlot.dynamic_plot. Args are optional (#3754)
• [ ] pybamm.QuickPlot.get_spatial_var. Should be private (name start with underscore) (#3754)
• [ ] pybamm.QuickPlot.plot. Document dynamic arg (#3754)
• [ ] pybamm.plot. Format bits of code as inline code, ax Arg should link to matplotlib Axis docs, kwarg arg should refer to matplotlib.pyplot.plot, not plt.plot. See also pybamm.plot2D
• [ ] pybamm.plot_voltage_components. Arg kwargs_fill, format ax.fill_between as code and link to matplotlib docs for this function.
• [ ] pybamm.plot_summary_variables. Arg kwargs_fig, format plt.subplots as code and link to matplotlib docs. I also think the actual name of this function shouldn't start with plt (presumably matplotlib.pyplot.subplots?)
• [ ] pybamm.get_git_commit_info. document return type
• [x] pybamm.rmse. document return type and args types
• [ ] pybamm.Timer. example is not properly formatted. In Timer.time, meth:reset() is not properly formated
• [ ] pybamm.FuzzyDict.copy. needs docstring
• [ ] pybamm.FuzzyDict. get_best_matches. document key arg
• [ ] pybamm.FuzzyDict. search. document args. format code properly
• [ ] pybamm.load. Needs documentation, unclear that it is just a thin wrapper around pickle.load, and filename arg is not documented
• [x] pybamm.install_jax. Document arguments arg, unclear on the type.
• [x] pybamm.have_jax and pybamm.is_jax_compatible, document return type (bool?)
• [x] pybamm.steps mention that drive cycles should start at t=0 (see #3612).

[ejfdickinson]

I'd widen the one relating to "experiment steps" to make clear that it feels like bad practice to give detailed documentation for a constructor pybamm.step._Step() that is not supposed to be called in user code. Just list the permitted value formats and kwargs that can be used in the pybamm.step.[...] options.

[ejfdickinson]

Generally make clear where classes implement __getitem__ and/or __setitem__ and so these are the expected calling patterns.

Especially, make explicit that pybamm.Solution implements a dict of pybamm.ProcessedVariable instances, so that the standard calling pattern is using __getitem__() on the pybamm.Solution and then using a method of the pybamm.ProcessedVariable class.

[ejfdickinson]

pybamm.ProcessedVariable documents the public attribute data as "the same as entries" but doesn't document entries.

[valentinsulzer]

We could look into Google Season of Docs for this https://developers.google.com/season-of-docs/docs/timeline

Seems like there is a lot to be done to follow best practices, not requiring domain knowledge. @ejfdickinson seems like you have lots of good suggestions, would you be available to mentor?

[ejfdickinson]

@tinosulzer Yes in principle, but can you suggest what the time commitment and overall working period would be?

My suggestions are all provoked empirically by using the library for different tasks! I made a suggestion in DM to @brosaplanella which I'll copy here on how to identify areas where documentation is misaligned with expected use patterns.

My recommendation for the best thing you can do is get a couple of students who are good at Python but have never used PyBaMM before, and have them reproduce a few modelling papers, especially those that don't use PyBaMM. Then just sit over their shoulders and work out what is clear and what is not.

Here "sit over their shoulders" is not to be understood literally! Just get people writing down where things are non-obvious, touching base with power users, and then working out why the "power use" implementation couldn't be reasoned from the docs.