Unified entry point for models and parameter sets

[santacodes]

Description

Modified the existing parameter_sets API and unified entry points to accommodate model entry points. This is an extension of the existing model entry points in the pybamm-cookie project. Relevant discussion.

Fixes # (issue)

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

Attention: Patch coverage is 98.24561% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.21%. Comparing base (3421fac) to head (81a6221).

Files with missing lines	Patch %	Lines
src/pybamm/dispatch/entry_points.py	98.14%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4490      +/-   ##
===========================================
- Coverage    99.22%   99.21%   -0.01%     
===========================================
  Files          302      303       +1     
  Lines        22821    22832      +11     
===========================================
+ Hits         22643    22653      +10     
- Misses         178      179       +1     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[arjxn-py]

Good start @santacodes, looks like this is still a WIP? Still I have commented just a tip hoping it'd make sense.

There are some conflicts that you may want to resolve and also feel free to add [WIP] prefix to your PR title while you're on it.

[arjxn-py]

Why skipping doctests?

[santacodes]

The model function returns an object of an initialised model along with the address of the object. This might vary per instance in a personal computer so the doctests couldn't assert the example I have mentioned in docstring. For example something like <pybamm.models.spm.SPM object 0x7381> is returned when the Model() function is called, the address might be different across various systems which would cause the doctests to fail. In brief I think doctesting and asserting two objects of a class would be a bad idea, unless there's a workaround for it. Do let me know if there is, I'm happy to implement it! :)

[arjxn-py]

Would Model() as a method be sufficient? My idea is that a class would add more utility and you can have similar required methods as parameter sets i.e. get_docstring or other model operations.

[santacodes]

All of the parameter_sets function also works for models. The Model() method is just an addition to that. As I mentioned this is a unified entry point for both models and parameter-sets. They operate on the same API, just different instances for models and parameter-sets.

[valentinsulzer]

this should just be called models, a parameter_set is a set of parameters (and a model is a set of equations), there is no set of models

[santacodes]

Yes that was intentional, models wasn't working because the pybamm.models namespace is already taken by the collection of battery models which gets overridden first. We could either change the variable name which was my temporary fix (model_sets) to something more sensible or we could change the namespace for the entry point models to something like pybamm.entrypoint.models. I'd prefer the first option though if you could come up with a sensible variable name for the models instance because I have a limited knowledge with the battery library.

[valentinsulzer]

Understood! In order to keep it consistent with parameter sets let's keep it as is then

[agriyakhetarpal]

How about moving both under pybamm.dispatch (internally, not exposed to users)? It's becoming fairly common with tools like networkx and scikit-image: https://scientific-python.org/specs/spec-0002/#projects-developing-a-python-entry_points-based-backend-dispatching

[valentinsulzer]

works for me!

[santacodes]

Sorry for the late response. I have abstracted the models instance inside the dispatch namespace as @agriyakhetarpal mentioned. I did not move the parameter_sets instance inside the dispatch namespace, as it would be a breaking change and also fail a lot of tests because pybamm.parameter_sets wouldn't work instead, people would have to use pybamm.dispatch.parameter_sets, though both of these work as of now. The models instance (entry point) can now be accessed through pybamm.dispatch.models and list(pybamm.dispatch.models) would list all the battery models available through the entry point. To load the model, pybamm.Model("SPM") would return the model object.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB