Skip to content
New issue

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

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

Already on GitHub? # to your account

Refactor submodels to use a single build method with no inputs #4594

Draft
wants to merge 40 commits into
base: develop
Choose a base branch
from

Conversation

aabills
Copy link
Contributor

@aabills aabills commented Nov 19, 2024

Description

This pull request refactors the external circuit submodel to use a single build method. It also sets up the code in BaseModel to use a build method, and to link coupled variables in the appropriate places.

Contributes to #3908

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

@aabills aabills added the skip release Do not merge until the next release label Nov 19, 2024
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

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

Project coverage is 99.24%. Comparing base (bcdd0b5) to head (4214fb1).
Report is 7 commits behind head on develop.

Files with missing lines Patch % Lines
src/pybamm/models/base_model.py 97.87% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4594      +/-   ##
===========================================
- Coverage    99.26%   99.24%   -0.03%     
===========================================
  Files          302      302              
  Lines        22889    22958      +69     
===========================================
+ Hits         22721    22784      +63     
- Misses         168      174       +6     

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


🚨 Try these New Features:

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@kratman kratman removed the skip release Do not merge until the next release label Nov 25, 2024
@aabills aabills changed the title Build method external circuit Refactor submodes to use a single build method with no inputs Nov 25, 2024
@aabills aabills changed the title Refactor submodes to use a single build method with no inputs Refactor submodels to use a single build method with no inputs Nov 25, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants