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.
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
Jd/radial branch reduction #1033
Jd/radial branch reduction #1033
Changes from 70 commits
54d707f
df8f361
fdba9ba
040b150
b8f9629
2deda19
d18aaea
c393f81
c8845f1
4d2c34c
fbcd7da
51260df
21b0ae4
8154c9a
d1dd6b1
492aee9
b77ecc5
c927047
954a916
8e960bd
37d05db
cfef00b
bf24077
f5e52bd
a8be7cf
a3af6b3
86e3fc8
b8cac7f
6976b14
7833677
4826380
32c7e03
8ac313f
4fa2a15
82b8115
4f50cc8
2c4d558
c1972c8
b50e02c
5aefd0a
1ee92de
2cb0c36
869042f
72ae721
4e9d046
b3d7511
8e1a85c
e1aab9e
55704f1
67cf974
b9195ea
1901f14
46122d5
f2a2a88
6ea9724
ed843d5
5983103
74a643c
d00ad50
32b40cb
f0a7c76
2f6b9f0
82865ab
a5764c3
0770442
f856011
5a5e9b8
4a5a05e
035ef78
2ab1dd4
c9a7b5a
3bb6e78
4c1ccbd
c92e073
c046be7
db9e04c
c6539e3
d2ba948
e356d1e
9f1c16d
782e017
f93f740
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 fully understand what
bus_reduction_map
is, but I'm a little wary of a completely different thing happening when it is empty. If an empty dictionary is meant to be a "null value" signifying that the bus reduction map should not be used, maybe making itnothing
(and editing the relevant zero-argument constructor, etc. accordingly) would be a better way to indicate this?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.
The bus reduction map keep track of where the components of a bus that has been eliminated a now connected. For instance if a bus A is radially connected to B and A was eliminated, what the map does is to establish the model won't include A and anything connected to A will be connected to B in the model
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.
This block appears three times and there is further code duplication in the corresponding three methods of
_make_system_expressions!
; maybe refactor with a helper functionThere 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.
yeah, with the years some methods have created code duplication. We need to budget for a hygiene clean up of the code base @claytonpbarrows
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.
Factor out the code duplication in the two methods of
branch_rate_bounds!
, the only difference is the varsThere 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.
Ditto above
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.
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.
lets leave this change for an overall search and replace. There are over 40 instances of this.
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.
Factor code duplication out of this and the other
add_constraints!
method to which this line is added, the two methods are nearly identical