-
Notifications
You must be signed in to change notification settings - Fork 72
Feature/example of drop=True #557
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
base: master
Are you sure you want to change the base?
Conversation
FabianHofmann
left a comment
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.
thanks for the pr @RobbieKiwi, one comment
linopy/expressions.py
Outdated
| def nvar(self) -> int: | ||
| """ | ||
| Get the number of unique variables in the linear expression. | ||
| Note that nvar <= nterm, as variables can appear multiple times and there can be terms which are completely masked out. |
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.
I don't think nvar is a good name here. in model.nvar this relates to the number of reference variables (each single element in a variable array). perhaps let's remove this function for now?
Closes #464
Changes proposed in this Pull Request
Add an example to the docs of
LinearExpression.where(..., drop=True)Add new property to base expression
BaseExpression.variable_namesNot addressed / further ideas
In the issue mentioned, I think the reporter wanted to see the unused terms in
LinearExpression.__repr__. After having a bit of a look at it, I think this is impossible because the term dim has no coordinate names. There seems to no way to know what variable originally belonged in a position of the term dim if the original variable has been completely masked (replaced with -1).Maybe when drop is True we could also run
.simplifyto get rid of any empty positions in the term dim?Currently
drop=Truewill drop unused index positions along all dimensions apart from the term dim, it might make more sense to drop on all dims?Checklist
doc.doc/release_notes.rstof the upcoming release is included.