-
-
Notifications
You must be signed in to change notification settings - Fork 549
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
Add base parameter class #4378
Open
kratman
wants to merge
33
commits into
pybamm-team:develop
Choose a base branch
from
kratman:feat/refactorParameters
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add base parameter class #4378
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
9447f76
Break up Chen2020
kratman 5d7bac4
Add new base class
kratman 3ac6988
Working on base parameter class
kratman 137be6a
Split thermal and converted lead-acid
kratman ab756f8
Style
kratman 97699ed
Added base class to all Li-ion models
kratman 9559363
Update Xu2019
kratman 98e536f
Update Ramadass2004
kratman 36980b6
Merge branch 'develop' of github.com:kratman/PyBaMM into feat/refacto…
kratman f248715
Update prada2013
kratman 9b60f84
Partial update to ORegan
kratman 56b1588
Prada cleanup
kratman 7ef8588
Update ORegan2022
kratman 242a34d
Finish Ai2020
kratman 7597c2b
Fixes
kratman 2020e91
Update chen composite
kratman 54d0e3f
Finish Ecker2015
kratman 0a6b506
Update Ecker2015Graphite
kratman e5bb634
Update Marquis2019
kratman 9107e27
Fix docstring test
kratman 96bd4dc
Finish Mohtat2020
kratman ce28934
Update Kim2011
kratman df6f53a
Update OKane2022
kratman 73d72f7
Update OKane2022 graphite siox halfcell
kratman 587ed57
Merge branch 'develop' of github.com:kratman/PyBaMM into feat/refacto…
kratman 82fcaaa
Minor docs updates
kratman 12f8e1f
Merge branch 'develop' of github.com:kratman/PyBaMM into feat/refacto…
kratman 02bbe32
Doc fixes
kratman d165a0e
Add tests
kratman 5fbfd8f
Remove redundant init
kratman 421ac28
Merge branch 'develop' into feat/refactorParameters
kratman 51c4978
Merge branch 'develop' into feat/refactorParameters
kratman 3848c7c
Merge branch 'develop' into feat/refactorParameters
kratman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
class AbstractBaseParameters: | ||
""" | ||
Base class for grouping and processing parameter sets. | ||
""" | ||
|
||
_details = {} | ||
_plating = {} | ||
_sei = {} | ||
_thermal = {} | ||
_cell = {} | ||
_negative_electrode = {} | ||
_positive_electrode = {} | ||
_separator = {} | ||
_electrolyte = {} | ||
|
||
def degradation_available(self): | ||
return True if self._sei else False | ||
|
||
def thermal_available(self): | ||
return True if self._thermal else False | ||
|
||
def plating_available(self): | ||
return True if self._plating else False | ||
|
||
def get_param_set(self): | ||
full_set = {} | ||
for sub_set in [ | ||
self._details, | ||
self._plating, | ||
self._sei, | ||
self._thermal, | ||
self._cell, | ||
self._negative_electrode, | ||
self._positive_electrode, | ||
self._separator, | ||
self._electrolyte, | ||
]: | ||
full_set.update(sub_set) | ||
return full_set |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it AbstractBaseParameters and not just BaseParameters? If anything, these would be battery parameters (rather than abstract parameters). Also, might be worth to use BaseParameterSet or BaseParameterValues instead. I think we are not 100% consistent with this, but in theory parameters are the concept (e.g. diffusion coefficient) while parameter values/sets are the value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Abstract as in an abstract class where implementation details are not fully defined. The idea was that this provides an interface that we can extend for the various parameter sets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for changing to
AbstractBaseParameterSets