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

Update AC/DC docs #153

Merged
merged 15 commits into from
Dec 18, 2024
Merged

Update AC/DC docs #153

merged 15 commits into from
Dec 18, 2024

Conversation

klamike
Copy link
Collaborator

@klamike klamike commented Dec 17, 2024

This branch continues from mt/ac-docs:

  • Switch to MathJax3 (MathJax is deprecated)
  • Utility functions for reusing our definitions.tex file
  • Slightly widened the docs-main div so the labels fit
  • Updated AC docs
  • Updated DC docs

Note this version doesn't have the corresponding dual next to each constraint / in the data table.

https://ai4opt.github.io/OPFGenerator/previews/PR153/opf/acp/

@klamike klamike requested a review from mtanneau December 17, 2024 18:19
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

see 1 file with indirect coverage changes

@klamike klamike changed the title Update AC docs Update docs Dec 17, 2024
@klamike klamike changed the title Update docs Update AC/DC docs Dec 17, 2024
@klamike klamike changed the base branch from mt/ac-docs to main December 17, 2024 18:47
docs/definitions.jl Outdated Show resolved Hide resolved
@mtanneau
Copy link
Contributor

@klamike should I close #151 or merge this branch into mine? If the latter, can you update the target branch for the PR please?

@klamike
Copy link
Collaborator Author

klamike commented Dec 17, 2024

@mtanneau If you like this one, close #151

@mtanneau
Copy link
Contributor

Note this version doesn't have the corresponding dual next to each constraint / in the data table.

The reason for indicating the dual variables in bracket is because I want to (eventually) write down the dual formulations as well.
I can do that in a separate PR, and it doesn't require indicating dual variables in the primal formulation... but it will (re)introduce the mathematical symbols for dual variables.

@klamike
Copy link
Collaborator Author

klamike commented Dec 17, 2024

Makes sense. Note our dual definitions are already in the definitions.tex so it shouldn't take long to add them.

docs/src/opf/acp.md Outdated Show resolved Hide resolved
## Mathematical Formulation

The ACOPF model considered in OPFGenerator is presented below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall comment on the model: we've encountered some issues with the sign of dual variables for constraints that are written like

@constraint(model, x + y == z)

where x, y, z are all JuMP.VariableRefs. Namely, JuMP may end up with x + y - z == 0 or z - x - y == 0 (I assume there is some consistency in how that transformation is done, but I don't know what it is). We ended up with dual variables that were negated compared to what we expected :(

I wrote down all the constraints with variables on the left and constant terms on the right to avoid that risk.
Again, no impact on the primal, but it can change the sign of dual variables, so I'd like to have as clear conventions as possible.

docs/src/assets/definitions.tex Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/dcp.md Outdated Show resolved Hide resolved
docs/src/opf/dcp.md Outdated Show resolved Hide resolved
klamike and others added 3 commits December 17, 2024 18:07
Co-authored-by: Mathieu Tanneau <9593025+mtanneau@users.noreply.github.com>
docs/src/assets/definitions.tex Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mtanneau mtanneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small notation change on entering/exiting arcs 🙏

docs/src/opf/dcp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
docs/src/opf/acp.md Outdated Show resolved Hide resolved
klamike and others added 2 commits December 17, 2024 18:33
Co-authored-by: Mathieu Tanneau <9593025+mtanneau@users.noreply.github.com>
Co-authored-by: Mathieu Tanneau <9593025+mtanneau@users.noreply.github.com>
@mtanneau
Copy link
Contributor

There remains one thing that I feel strongly about: writing the constraints with only constant terms in the RHS.
I feel strongly about it because it's been an issue for us in the past when working on dual formulations.

I can make the change in the same PR as when I add the dual formulations.

Copy link
Contributor

@mtanneau mtanneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtanneau is in charge of adding the dual stuff in a later PR

@mtanneau mtanneau merged commit 8438fae into main Dec 18, 2024
4 checks passed
@klamike klamike deleted the mk/update-ac-docs branch December 18, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants