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

Make summary variables an attribute of the model #1726

Closed
rtimms opened this issue Oct 7, 2021 · 2 comments · Fixed by #1760
Closed

Make summary variables an attribute of the model #1726

rtimms opened this issue Oct 7, 2021 · 2 comments · Fixed by #1760
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours

Comments

@rtimms
Copy link
Contributor

rtimms commented Oct 7, 2021

At the moment the simulation stores some summary variables, but these are hardcoded into solution. It would be nice if the list of summary variables were defined on the model so that different models could have different summary variables, or users could define their own list/add their own variables. Might need some checks (e.g. need to be scalars).

@valentinsulzer
Copy link
Member

valentinsulzer commented Oct 8, 2021

Yeah this is a good idea. Pretty easy, just make this list an attribute of the model? And have the others (minQ/maxQ, eSOH variables) hardcoded

@valentinsulzer valentinsulzer added the difficulty: easy A good issue for someone new. Can be done in a few hours label Oct 8, 2021
@valentinsulzer
Copy link
Member

When doing so, also need to check that every variable in the list is also a variable of the model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants