Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Getting Started notebooks #3750

Merged
merged 34 commits into from
Mar 12, 2024
Merged

Conversation

brosaplanella
Copy link
Sponsor Member

@brosaplanella brosaplanella commented Jan 20, 2024

Description

This PR aims to improve and simplify the Getting Started notebooks so they are a better resource to new users (I am assuming users could have very limited Python knowledge). It is still work in progress.

Main changes

  • Tutorial 1: minor text changes.
  • Tutorial 2: minor text changes.
  • Tutorial 3: removed the part where two simulations (DFN & SPM) got the voltage components plot together as it used some additional plotting functionality outside of PyBaMM. Also implemented the new API for plot_voltage_components which matches the functionality of plot.
  • Tutorial 4: removed the drive cycle example, as I believe we should encourage users to use the experiment class for that. This also means we do not need to import os for this notebook.
  • Tutorial 5: minor text changes plus extra detail added on the cycle and step definition.
  • Tutorial 6: minor text changes.
  • Tutorial 7: minor text changes plus list of notebooks with other examples.
  • Tutorial 8: minor text changes, including the fact that the fast solver postprocess the solution and trims the interval past the cut-off voltage (so the discussion was wrong).
  • Tutorial 9: minor text changes.
  • Tutorial 10: I wanted to move it to Creating Models folder, but it seems a duplication of 3-negative-particle-problem.ipynb` so I deleted it.
  • Tutorial 11: moved to Creating Models folder (which will be addressed at a later stage).

Questions

  • Tutorial 4: do we want to show how to define your own parameter set as a file? I would say no (or at least not in this notebook),

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

codecov bot commented Jan 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (bc31cac) to head (2dd7baa).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3750   +/-   ##
========================================
  Coverage    99.60%   99.60%           
========================================
  Files          259      259           
  Lines        21270    21270           
========================================
  Hits         21186    21186           
  Misses          84       84           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brosaplanella
Copy link
Sponsor Member Author

Addressed the comments and fixed a small typo in the parameter argument of current_with_time.

Copy link
Contributor

@kratman kratman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@agriyakhetarpal
Copy link
Member

I can leave a review later in the day today; adding a note to self that broken links should be checked across the documentation and that the links on https://pybamm.org/learn/ would have to be updated in accordance with these changes

@kratman
Copy link
Contributor

kratman commented Feb 27, 2024

@brosaplanella Just noticed these in the doc tests. Missed it before

/home/runner/work/PyBaMM/PyBaMM/docs/source/user_guide/index.md:45: WARNING: toctree contains reference to nonexisting document 'source/examples/notebooks/getting_started/tutorial-10-creating-a-model'
/home/runner/work/PyBaMM/PyBaMM/docs/source/user_guide/index.md:45: WARNING: toctree contains reference to nonexisting document 'source/examples/notebooks/getting_started/tutorial-11-creating-a-submodel'
/home/runner/work/PyBaMM/PyBaMM/docs/source/examples/index.rst:10: WARNING: toctree contains reference to nonexisting document 'source/examples/notebooks/getting_started/tutorial-10-creating-a-model'
/home/runner/work/PyBaMM/PyBaMM/docs/source/examples/index.rst:10: WARNING: toctree contains reference to nonexisting document 'source/examples/notebooks/getting_started/tutorial-11-creating-a-submodel'
looking for now-outdated files... none found
pickling environment... done
checking consistency... /home/runner/work/PyBaMM/PyBaMM/docs/source/examples/notebooks/creating_models/7-creating-a-submodel.ipynb: WARNING: document isn't included in any toctree

@kratman
Copy link
Contributor

kratman commented Mar 5, 2024

@brosaplanella Is this ready to go now?

Copy link
Member

@valentinsulzer valentinsulzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks, would just suggest giving more info on adding your own parameter set in tutorial 4

@brosaplanella
Copy link
Sponsor Member Author

Should I add a short paragraph explaining that the option exists and pointing to the example, or did you mean a fully fleshed example?

@valentinsulzer
Copy link
Member

valentinsulzer commented Mar 5, 2024

Something that says you can create custom parameter values by passing in a dictionary, with an example including a couple of scalars and a function, e.g.

def cubed(x):
    return x ** 3

parameter_values = pybamm.ParameterValues({
    "a": 1,
    "b": 2,
    "c": cubed,
})
  • explain that the function object itself should be passed in without being called

Then provide a link to the folder of .py files of parameters where people can find an example to copy for their own custom parameter sets

@kratman
Copy link
Contributor

kratman commented Mar 11, 2024

@brosaplanella I will re-review this afternoon so we can get these updates in there

@kratman kratman merged commit 5fa1070 into develop Mar 12, 2024
46 checks passed
@kratman kratman deleted the improve-getting-started-notebooks branch March 12, 2024 18:41
agriyakhetarpal pushed a commit to pybamm-team/pybamm.org that referenced this pull request Mar 12, 2024
The links to deleted notebooks in `learn.md` now have been removed.
Related to pybamm-team/PyBaMM#3750
lorenzofavaro pushed a commit to lorenzofavaro/PyBaMM that referenced this pull request Mar 13, 2024
* improve tutorials 1 to 3

* use new syntax for plot_voltage_components

* style: pre-commit fixes

* simplify notebook by removing drive cycle

* add additional comment on dynamic_plot

* improve tutorial 5

* remove the use of solution in notebook 5, this is introduced in notebook 6

* fix minor typo

* minor text updates

* style: pre-commit fixes

* minor text updates plus extra links

* minor text updates!

* minor text updates

* remove tutorial 10 as very similar to tutorial 3 in creating models

* move and rename tutorial 11 to creating models section

* style: pre-commit fixes

* fix minor typo in current_with_time

* implement Rob & Eric's comments

* update docs links

* Valentin's comments

* style: pre-commit fixes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
agriyakhetarpal added a commit that referenced this pull request Mar 13, 2024
* Enable flake8-bugbear

Fix unused-loop-control-variable (B007)

Fix function-uses-loop-variable (B023)

Fix raise-without-from-inside-except (B904)

Fix mutable-argument-default (B006)

Fix no-explicit-stacklevel (B028)

Fix get-attr-with-constant (B009)

Fix loop-variable-overrides-iterator (B020)

Fix unary-prefix-increment-decrement (B002)

Fix function-call-in-default-argument (B008)

Fix cached-instance-method (B019)

* Show exception chain (B904)

* Disable single lint check (B019)

* delay xarray.DataArray initialization

* revert example

* Bump the actions group with 1 update (#3861)

Bumps the actions group with 1 update: [awalsh128/cache-apt-pkgs-action](https://github.com/awalsh128/cache-apt-pkgs-action).


Updates `awalsh128/cache-apt-pkgs-action` from 1.4.1 to 1.4.2
- [Release notes](https://github.com/awalsh128/cache-apt-pkgs-action/releases)
- [Commits](awalsh128/cache-apt-pkgs-action@v1.4.1...v1.4.2)

---
updated-dependencies:
- dependency-name: awalsh128/cache-apt-pkgs-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make `typing-extensions` and `sympy` required dependencies to fix PyBaMM import (#3848)

* import `typing_extensions` as optional_dependency

* Make `typing-extensions` a required dependency

* Try forward referencing for `sympy` in `IndependentVariable`

* Make `sympy` a required dependency

* Update docs for SymPy

* Import `sympy` without `have_optional_dependency`

* Import `sympy` without `TYPE_CHECKING`

* Changelog

* Update CHANGELOG.md

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Rearrange position in changelog

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>

* Rename have_optional_dependency (#3866)

* Rename have_optional_dependency

* Change log

* Fix import

* style: pre-commit fixes

* Update pybamm/util.py

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>

* update changelog for #3862 (#3869)

* update changelog for #3862

* reword

* fix deprecation warning and check for electrode/particle diffusivity

* update test

* Edit naming for unused loop control variable (B007)

* #3883 updates to install from source (#3884)

* Disable benchmarks on PRs (#3876)

* Remove`[ latexify` extra (#3888)

* Improve Getting Started notebooks (#3750)

* improve tutorials 1 to 3

* use new syntax for plot_voltage_components

* style: pre-commit fixes

* simplify notebook by removing drive cycle

* add additional comment on dynamic_plot

* improve tutorial 5

* remove the use of solution in notebook 5, this is introduced in notebook 6

* fix minor typo

* minor text updates

* style: pre-commit fixes

* minor text updates plus extra links

* minor text updates!

* minor text updates

* remove tutorial 10 as very similar to tutorial 3 in creating models

* move and rename tutorial 11 to creating models section

* style: pre-commit fixes

* fix minor typo in current_with_time

* implement Rob & Eric's comments

* update docs links

* Valentin's comments

* style: pre-commit fixes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>

* chore: update pre-commit hooks (#3890)

* chore: update pre-commit hooks

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.2.2 → v0.3.2](astral-sh/ruff-pre-commit@v0.2.2...v0.3.2)

* style: pre-commit fixes

* Update .git-blame-ignore-revs

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* style: pre-commit fixes

* Resolve conflicts (B904, B028, B020)

* Added a schedule on `needs-reply remove` workflow  (#3891)

* added schedule to need-reply workflow

* added schedule to need-reply workflow

---------

Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>

* style: pre-commit fixes

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Martin Robinson <martinjrobins@gmail.com>
Co-authored-by: Ferran Brosa Planella <Ferran.Brosa-Planella@warwick.ac.uk>
Co-authored-by: Santhosh <52504160+santacodes@users.noreply.github.com>
js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this pull request Aug 12, 2024
* improve tutorials 1 to 3

* use new syntax for plot_voltage_components

* style: pre-commit fixes

* simplify notebook by removing drive cycle

* add additional comment on dynamic_plot

* improve tutorial 5

* remove the use of solution in notebook 5, this is introduced in notebook 6

* fix minor typo

* minor text updates

* style: pre-commit fixes

* minor text updates plus extra links

* minor text updates!

* minor text updates

* remove tutorial 10 as very similar to tutorial 3 in creating models

* move and rename tutorial 11 to creating models section

* style: pre-commit fixes

* fix minor typo in current_with_time

* implement Rob & Eric's comments

* update docs links

* Valentin's comments

* style: pre-commit fixes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this pull request Aug 12, 2024
* Enable flake8-bugbear

Fix unused-loop-control-variable (B007)

Fix function-uses-loop-variable (B023)

Fix raise-without-from-inside-except (B904)

Fix mutable-argument-default (B006)

Fix no-explicit-stacklevel (B028)

Fix get-attr-with-constant (B009)

Fix loop-variable-overrides-iterator (B020)

Fix unary-prefix-increment-decrement (B002)

Fix function-call-in-default-argument (B008)

Fix cached-instance-method (B019)

* Show exception chain (B904)

* Disable single lint check (B019)

* delay xarray.DataArray initialization

* revert example

* Bump the actions group with 1 update (pybamm-team#3861)

Bumps the actions group with 1 update: [awalsh128/cache-apt-pkgs-action](https://github.com/awalsh128/cache-apt-pkgs-action).


Updates `awalsh128/cache-apt-pkgs-action` from 1.4.1 to 1.4.2
- [Release notes](https://github.com/awalsh128/cache-apt-pkgs-action/releases)
- [Commits](awalsh128/cache-apt-pkgs-action@v1.4.1...v1.4.2)

---
updated-dependencies:
- dependency-name: awalsh128/cache-apt-pkgs-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make `typing-extensions` and `sympy` required dependencies to fix PyBaMM import (pybamm-team#3848)

* import `typing_extensions` as optional_dependency

* Make `typing-extensions` a required dependency

* Try forward referencing for `sympy` in `IndependentVariable`

* Make `sympy` a required dependency

* Update docs for SymPy

* Import `sympy` without `have_optional_dependency`

* Import `sympy` without `TYPE_CHECKING`

* Changelog

* Update CHANGELOG.md

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Rearrange position in changelog

---------

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>

* Rename have_optional_dependency (pybamm-team#3866)

* Rename have_optional_dependency

* Change log

* Fix import

* style: pre-commit fixes

* Update pybamm/util.py

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>

* update changelog for pybamm-team#3862 (pybamm-team#3869)

* update changelog for pybamm-team#3862

* reword

* fix deprecation warning and check for electrode/particle diffusivity

* update test

* Edit naming for unused loop control variable (B007)

* pybamm-team#3883 updates to install from source (pybamm-team#3884)

* Disable benchmarks on PRs (pybamm-team#3876)

* Remove`[ latexify` extra (pybamm-team#3888)

* Improve Getting Started notebooks (pybamm-team#3750)

* improve tutorials 1 to 3

* use new syntax for plot_voltage_components

* style: pre-commit fixes

* simplify notebook by removing drive cycle

* add additional comment on dynamic_plot

* improve tutorial 5

* remove the use of solution in notebook 5, this is introduced in notebook 6

* fix minor typo

* minor text updates

* style: pre-commit fixes

* minor text updates plus extra links

* minor text updates!

* minor text updates

* remove tutorial 10 as very similar to tutorial 3 in creating models

* move and rename tutorial 11 to creating models section

* style: pre-commit fixes

* fix minor typo in current_with_time

* implement Rob & Eric's comments

* update docs links

* Valentin's comments

* style: pre-commit fixes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>

* chore: update pre-commit hooks (pybamm-team#3890)

* chore: update pre-commit hooks

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.2.2 → v0.3.2](astral-sh/ruff-pre-commit@v0.2.2...v0.3.2)

* style: pre-commit fixes

* Update .git-blame-ignore-revs

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* style: pre-commit fixes

* Resolve conflicts (B904, B028, B020)

* Added a schedule on `needs-reply remove` workflow  (pybamm-team#3891)

* added schedule to need-reply workflow

* added schedule to need-reply workflow

---------

Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>

* style: pre-commit fixes

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Martin Robinson <martinjrobins@gmail.com>
Co-authored-by: Ferran Brosa Planella <Ferran.Brosa-Planella@warwick.ac.uk>
Co-authored-by: Santhosh <52504160+santacodes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants