refactor: simplify _build_model by defining functions that add constraints

[danielolsen]

Pull Request doc

Purpose

The _build_model function is currently 360 lines long, and will only get more complicated as we add more functionality. This PR simplifies this function by moving logic to add constraints to a model to individual functions. The end result is that the length of _build_model is reduced to 200 lines. This can serve other refactoring efforts, e.g. #60, as well as improve the test coverage, since now it is easier to write unit tests to ensure that each constraint or set of constraints is being build properly.
Closes #117.

Separately, we also implement a fix to the demand flexibility implementation, which I found during testing.

What the code is doing

We add new functions:

• _add_constraint_power_balance!
• _add_constraint_load_shed!
• _add_constraints_storage_operation!
• _add_constraints_demand_flexibility!
• _add_constraints_initial_ramping!
• _add_constraints_ramping!
• _add_constraints_generator_segments!
• _add_constraints_branch_flow_limits!
• _add_branch_angle_constraints!
• _add_profile_generator_limits!
• _add_objective_function!

Per Julia convention, these function names end with ! since they modify at least one of the inputs (the JuMP model).

Within these functions, we make use of the JuMP syntax model[:foo] to access the variable or contraint (or array of variables/contraints) named foo that was defined in the model object. This way, we can simplify passing the model object back and forth, and don't have to manually plumb the foo references that get created in the _build_model namespace.

Testing

Tested manually with storage, load shedding, and demand flexibility. Given the scope of this refactor, addition testing by other people is highly appreciated.

Usage Example/Visuals

Usage from user-facing functions is unchanged.

Time estimate

>= 1 hour. There is still work to be done in improving the format of the code, but the main thing I'm looking for from reviewers at this draft stage is an assessment of the organization.

[danielolsen]

@dongqi-wu I think your feedback here would also be useful, since you'll be working with this codebase. I am agnostic as to the merge order between this PR and #135; if yours gets merged first, I can rebase around your changes without too much work.

[BainanXia]

This makes perfect sense to me and the logic is clean. Will read through the code once the PR is formally setup. Thanks.

[danielolsen]

I've made some changes to the code style to make it a bit easier to read. I haven't done too much research, but it seems like a Julia analog to python's black code style is Blue: https://github.com/invenia/BlueStyle, and I've made some change that I think conform to that standard. We will probably want a more thorough style cleanup later for the rest of the codebase.

I think this is now ready for a full review.

[dongqi-wu]

@dongqi-wu I think your feedback here would also be useful, since you'll be working with this codebase. I am agnostic as to the merge order between this PR and #135; if yours gets merged first, I can rebase around your changes without too much work.

Hi Daniel,

I have looked at the changes, I think is a very good idea to separate build_model into individual functions so it becomes easier to change and test them. For my PR, I have addressed and tested Lane's comments locally and I will override my commit on PR soon. We can discuss it Tuesday, if there is no more issues for the new version we can probably merge it shortly after the meeting.

[BainanXia]

@dongqi-wu I think your feedback here would also be useful, since you'll be working with this codebase. I am agnostic as to the merge order between this PR and #135; if yours gets merged first, I can rebase around your changes without too much work.

Hi Daniel,

I have looked at the changes, I think is a very good idea to separate build_model into individual functions so it becomes easier to change and test them. For my PR, I have addressed and tested Lane's comments locally and I will override my commit on PR soon. We can discuss it Tuesday, if there is no more issues for the new version we can probably merge it shortly after the meeting.

Hi Dongqi,

Thanks for the update. Please hold off overriding you commit on the open PR. We haven't heard from you last week and thought you didn't have access to Git in China. @lanesmith has prepared a draft PR based on your branch to address his comments and we are going to talk about it during the working session right after our next meeting (we will go through your local changes as well). Ideally, we can get it merged during the working session.

[dongqi-wu]

@dongqi-wu I think your feedback here would also be useful, since you'll be working with this codebase. I am agnostic as to the merge order between this PR and #135; if yours gets merged first, I can rebase around your changes without too much work.

Hi Daniel,
I have looked at the changes, I think is a very good idea to separate build_model into individual functions so it becomes easier to change and test them. For my PR, I have addressed and tested Lane's comments locally and I will override my commit on PR soon. We can discuss it Tuesday, if there is no more issues for the new version we can probably merge it shortly after the meeting.

Hi Dongqi,

Thanks for the update. Please hold off overriding you commit on the open PR. We haven't heard from you last week and thought you didn't have access to Git in China. @lanesmith has prepared a draft PR based on your branch to address his comments and we are going to talk about it during the working session right after our next meeting (we will go through your local changes as well). Ideally, we can get it merged during the working session.

Hi Bainan,

Sure, I'm sorry that I didn't let you know in advance that I was already working on the comments. I already have a VPN through which I can access anything including git. We can compare our change on Tuesday and hopefully merge the better version.

[lanesmith]

Overall, I think it looks great! _build_model is definitely much cleaner. After addressing my comments, I don't see a reason that this shouldn't be approved, though I'll give the other reviewers more time.

[BainanXia]

Still getting familiar with Julia syntax though. Reading through the code, the logic makes sense to me. Any final comments @lanesmith ?

[lanesmith]

Looks good to me too! Thank you for addressing my comments