Discover Parameter Sets using Entry Points #2396
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov ReportBase: 99.66% // Head: 99.72% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #2396 +/- ##
===========================================
+ Coverage 99.66% 99.72% +0.05%
===========================================
Files 269 268 -1
Lines 20190 20149 -41
===========================================
- Hits 20123 20093 -30
+ Misses 67 56 -11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@tinosulzer Can we make if so, we can replace change this: To: all_parameter_sets = [k for k,v in pybamm.parameter_sets.items() if v["chemistry"] == "lithium_ion"]Probably also need to update the docs for Adding Parameter Values to:
|
Building on #2342, this PR uses entry points to register and find new ParameterSets. This enables third-parties to distribute new parameter sets without opening a PR against PyBaMM. - Enables a decentralized approach to parameter sharing - Parameter sets provided by external packages can now be version controlled - User can now (once available) just `pip install` new parameter sets Both third-party and existing parameter sets are discovered via the `pybamm_parameter_sets` entry point, putting both on equal footing. Added the `get_docstring` method to ParameterSets to enable programmatic access to a parameters set's documentation.
Instead of documenting parameter sets in `pybamm/parameters/parameter_sets.py` document them at the point of declaration (i.e. at the entry point)
Now that parameters sets can come from outside of PyBamm, we can't rely
on pybamm/CITATIONS.txt as the sole source of citations.
This commit allows the `key` to either be a known citation key or a
bibtex formatted citation.
Supporting changes
- `_all_citations` now is a dict mapping citation keys to BibTex
strings. This lets us add new citations entries as needed.
- `register()` can cite multiple entries if a BibTex string with
multiple entries is added.
- Added `_add_citation` to handle adding new entries / overwriting old
ones if a duplicate key is provided
Breaking: Registering a citation can now overwrite a previous citation.
For example, the following will replace the citation for `Sulzer2019physical`
with the new BibTex citation globally.
```python
import pybamm
pybamm.citation.register("Sulzer2019physical")
pybamm.citations.register("@book{Sulzer2019physical, title={New}}")
```
This should be tolerable as:
- Sane citation keys should generally not cause duplicates
- Duplicate citation keys are generally not supported, better to warn,
then print out a BibTex file that could silently hide the duplication
- We warn the user when it happens
Doc Fixes: - missing black lines after explicit ref tag - Use labeled reference for link to pybamm issue template - Doctest for parameter sets was missing `<BLANKLINE>` callout, added - Doctest for citations failed to parse multi-line command, made single line Testing: - On windows, the NamedTemporaryFile get's deleted on first close. Fixed `test_citations.py` to manually delete the file instead. - Using `list[str]` as a type hint is unsupported in python 3.8, removed Coverage: - Missing tests for `pybamm.Citations._add_citation` - Missing tests for `__len__`, `__iter__` and `get_docstring` for `pybamm.parameters.parameter_sets.ParameterSets` -> added - Added test to ensure all parameter sets are registered via entry points
Per conversation with @tinosulzer, removed: - update_parameter_sets_docs.yml - No longer needed due to jinja - add-parameter-values.rst - Out of date (This PR + #2342) Fixed `examples/scripts/print_model_parameter_combinations.py` to use the dict interface to `pybamm.parameter_sets`
Needed to ensure the doc strings are formatted correctly
Fix typo in test_citations
|
@all-contributors add @awadell1 for code, tests, docs |
|
@tinosulzer I've put up a pull request to add @awadell1! 🎉 |
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Enables pybamm to discover parameter sets via the
pybamm_parameter_setentry point:As a bonus, updated
pybamm.Citationsto support registering BibTex strings, as external parameter sets don't have access topybamm/CITATIONS.txtcfcb589: This is mostly superseded by c2be58d, which uses jinja +
pybamm.parameter_sets.get_docstringto generate the docstrings for all parameter sets. On the plus side, this supports all the fun sphinx documentation features. The downside the current doc strings are not formatted as reStructuredText. I'm happy to fix that but wanted to get the ball rolling first.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.
Key checklist:
$ flake8$ python run-tests.py --unit$ cd docsand then$ make clean; make htmlYou can run all three at once, using
$ python run-tests.py --quick.Further checks: