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

Proposed fixes / consistency changes for documentation #643

Open
sstroemer opened this issue Jul 26, 2024 · 7 comments · May be fixed by #700
Open

Proposed fixes / consistency changes for documentation #643

sstroemer opened this issue Jul 26, 2024 · 7 comments · May be fixed by #700
Assignees
Milestone

Comments

@sstroemer
Copy link
Contributor

sstroemer commented Jul 26, 2024

Description

Follow up after a discussion with @sjpfenninger on 18.07., regarding some findings after combing through the current documentation, in no particular order (just numbered to make references in discussion easier):

  1. lookup_cluster_first_time occurs in the base math; should that be part of the Inter-cluster storage math? [link]
  2. Missing links between entries:
    • flow_cap [link] makes use of, e.g., flow_cap_min, but the latter does not show that in it's Used in section [link]
    • other parameters may be missing too, I assume these relations are built automatically, so probably just a single fix necessary
  3. Adding "backlinks" between entries: area_use_max is used in area_use, which is easy to follow. The latter makes use of source_unit, which may be hard to see on a first look, and further is then "hard" to follow up on. I assume the "used in" section is built automatically, could the same approach also be used to build a "makes use of" section? That would
    • show all used parameters, etc. immediately in a single list, and
    • make it easy to traverse the documentation by just hopping along the "dependency graph".
  4. balance_storage checks for where: (include_storage=true or base_tech=storage) ..., whereas others check for where: storage ... instead; it seems to me those should be equivalent, because storage only exists with exactly the first condition [link] - if that is true this could be unified for consistency, if not, the difference in behavior could be documented
  5. flow_cap_min [link] contains a note stating "this will force flow_cap to a minimum value unless cap_method is set to integer", which seems to not relate to the definition of flow_cap in a setting where cap_method == LP (and maybe more to flow_cap_per_unit, etc.).
  6. Defaults, like for cyclic_storage, are hard to find: The docs do not mention this, but it can be checked, or found in the schema description. Feels like the base math section should (?) contain all these.
  7. Is there a way to alphabetically sort the entries in the right side-bar of the base math section? That would make it easier to find specific entries.

Related links

Links are included in the description, a few things that I could guess how to fix are in #644.

Version

latest (4fc6b84)

Proposed change

No response

@brynpickering
Copy link
Member

lookup_cluster_first_time occurs in the base math; should that be part of the Inter-cluster storage math? [link]

No, if clustering the timeseries then it is needed irrespective of whether inter-cluster storage is used.

Missing links between entries:
flow_cap [link] makes use of, e.g., flow_cap_min, but the latter does not show that in it's Used in section [link]
other parameters may be missing too, I assume these relations are built automatically, so probably just a single fix necessary

They are there, e.g.:

image

Adding "backlinks" between entries: area_use_max is used in area_use, which is easy to follow. The latter makes use of source_unit, which may be hard to see on a first look, and further is then "hard" to follow up on. I assume the "used in" section is built automatically, could the same approach also be used to build a "makes use of" section? That would
show all used parameters, etc. immediately in a single list, and
make it easy to traverse the documentation by just hopping along the "dependency graph".

Yeah, it could be done automatically, just needs a way to store that information so the page doesn't explode even more than it is.

balance_storage checks for where: (include_storage=true or base_tech=storage) ..., whereas others check for where: storage ... instead; it seems to me those should be equivalent, because storage only exists with exactly the first condition [link] - if that is true this could be unified for consistency, if not, the difference in behavior could be documented

Yep, they are the same and could be boiled down to include_storage=True.

flow_cap_min [link] contains a note stating "this will force flow_cap to a minimum value unless cap_method is set to integer", which seems to not relate to the definition of flow_cap in a setting where cap_method == LP (and maybe more to flow_cap_per_unit, etc.).

I'm a bit confused by this. If cap_method=continuous for a given technology then flow_cap>=flow_cap_min. If cap_method=integer then flow_cap=0|flow_cap>=flow_cap_min.

Defaults, like for cyclic_storage, are hard to find: The docs do not mention this, but it can be checked, or found in the schema description. Feels like the base math section should (?) contain all these.

Yeah, I'm planning to move parameter metadata to the math (so they aren't all hidden in the schema). Then we could show them easily in the docs.

Is there a way to alphabetically sort the entries in the right side-bar of the base math section? That would make it easier to find specific entries.

Certainly would, but not possible in the docs engine. We could sort parameters/constraints/variables alphabetically in the source files. I prefer not to do the same with global expressions as their order is important (you can't reference another global expression unless it comes earlier in the set of definitions).

@sstroemer
Copy link
Contributor Author

Thanks for the in-depth answer!

No, if clustering the timeseries then it is needed irrespective of whether inter-cluster storage is used.

Yeah, that makes sense, I kinda misread the formulation and that got me confused.

They are there, e.g.:

I have no idea how I messed that up... I'm sorry! I somehow had some tabs in a slightly outdated version (635) and didn't realize this. I even linked to the version where it's correct 🤦‍♂️

Yeah, it could be done automatically, just needs a way to store that information so the page doesn't explode even more than it is.

Maybe a standard "details admonition"? Seems like the mkdocs.yml activates pymdownx.details already, so maybe that (or the upcoming blocks alternative) could be an option? Could also make the already existing long lists (e.g., flow_cap) more tractable.

I'm a bit confused by this. If cap_method=continuous for a given technology then flow_cap>=flow_cap_min. If cap_method=integer then flow_cap=0|flow_cap>=flow_cap_min.

I'll reply to that separately, I need to think that one through at a less-tired-brain time.

@sjpfenninger sjpfenninger added this to the 0.7.0 milestone Aug 6, 2024
@sjpfenninger sjpfenninger self-assigned this Aug 6, 2024
@sstroemer
Copy link
Contributor Author

sstroemer commented Aug 6, 2024

Preface (!): All of this is "unimportant" for me, no reason to prioritize, it's just what I noticed and it's in no way a problem for me right now.

I'm having a hard time wrapping my head around this, so sorry for the underlying confusion... I'll try anyways, although it feels like I'm overlooking something.

I'm a bit confused by this. If cap_method=continuous for a given technology then flow_cap>=flow_cap_min. If cap_method=integer then flow_cap=0|flow_cap>=flow_cap_min.


Documentation

My brain translates "[...] this will force flow_cap to a minimum value unless cap_method is set to integer [...]" to:

  • "unless [...]" => if cap_method != integer, then
  • force the variable flow_cap to a THIS minimum value (because this is the thing I'm currently reading, flow_cap_min), meaning flow_cap = flow_cap_min(as in "force")

Why? Because I already know, that flow_cap_min is a "limit" for flow_cap (my interpretation: the lower bound of a variable) from the first sentence, "Limits flow_cap to a minimum.". And I know that this lower bound is scaled in the case of cap_method=integer (last sentence, and the linked constraint flow_cap >= flow_cap_min * purchased_units).

So my confusion comes from: Either the note with the italic highlight is redundant (based on the first sentence), which I would not assume (otherwise, why point that out?), or force means something more serious than "the lower bound is as stated". But then I failed to find any obvious relation between cap_method and this interpretation?

Am I correct, that your quote above (| = $\vee$) is meant as:

  1. if cap_method == continuous then flow_cap >= flow_cap_min
  2. if cap_method == integer then flow_cap = 0 OR flow_cap >= flow_cap_min

However, what I get from the docs is along the lines of:

  1. if cap_method == continuous then flow_cap >= flow_cap_min
  2. if cap_method == integer then flow_cap >= flow_cap_min * purchased_units

I'm unsure where the part of flow_cap being forced to 0 comes from? (even if that may be tied to operating_units, that only exists for integer_dispatch and not based on cap_method alone).

Distortion of purchased_units

  1. Let's assume flow_cap_min > 0, and flow_cap_max > flow_cap_min.
  2. We know flow_cap >= flow_cap_min.
  3. Due to flow_capacity_max_purchase_milp, we know flow_cap <= flow_cap_max * purchased_units.

Combining (2.) and (3.) we get flow_cap_min <= flow_cap <= flow_cap_max * purchased_units, which requires purchased_units > 0 to be feasible.

Is that on purpose? I had the feeling no, since stuff like available_flow_cap_max_binary_continuous_switch exists to work around a similar thing, just for available_flow_cap.

available_flow_cap

I failed figuring that one out from the docs, so just posting current question:

Assuming flow_cap_max is set, and operating_units exists (so integer_dispatch = true, therefore cap_method = integer must be set, and both operating_units and purchased_units exists), then:

  1. available_flow_cap_continuous sets flow_cap as upper bound.
  2. available_flow_cap_binary enforces the second upper bound as flow_cap_max * operating_units, based on whether or not any units are actually operating.
  3. operating_units is bounded by purchased_units.
  4. flow_cap is already bounded by flow_cap_max * purchased_units.

So (2.) sets a bound on available_flow_cap (using operating_units), that is tighter than the one (1.) constructs, since that in the end comes down to purchased_units. This makes me think, this actually imposes a lower bound on flow_cap, meaning it could be interpreted as caring about flow_cap >= available_flow_cap? Or is that because flow_cap is actually the continuous ("variable") cost associated with sizing the unit? But that would only work for units which have binary investment, which these do not need to have? (and here is where I'm getting lost)

@sstroemer
Copy link
Contributor Author

sstroemer commented Aug 6, 2024

Refactored comment into separate issues #651 and #652.

@brynpickering
Copy link
Member

@sstroemer: this issue is becoming a bit of a monster. Can you please split it to make discussion easier?

Just a brief note on flow_cap_min, since we're going in circles here.

Am I correct, that your quote above (| = ∨ ) is meant as:
if cap_method == continuous then flow_cap >= flow_cap_min
if cap_method == integer then flow_cap = 0 OR flow_cap >= flow_cap_min

Yes.

What I am trying to get across in the description is that if you set flow_cap_min when cap_method=continuous, that technology will always exist in the model result, with at least the capacity set by flow_cap_min. If cap_method=integer, the technology may not exist in the model result, but if it does then its capacity will be at least flow_cap_min. The reason this is explicitly mentioned in the description is that users often expect the result of cap_method=integer to be the case when cap_method=continuous.

Let's assume flow_cap_min > 0, and flow_cap_max > flow_cap_min.
We know flow_cap >= flow_cap_min.
Due to flow_capacity_max_purchase_milp, we know flow_cap <= flow_cap_max * purchased_units.

You're missing that flow_cap >= flow_cap_min * purchased_units (flow_capacity_min_purchase_milp)

available_flow_cap

(1) and (2) are part of a 3-part constraint (along with available_flow_cap_max_binary_continuous_switch) that allows you to effectively multiply a binary decision variable with a continuous one. It enables "if operating units == 0, available capacity == 0. If operating units > 0, available capacity == flow capacity". I see now that it likely needs another constraint as it only works if purchased_units has a maximum of 1, i.e. it is a binary decision variable. Because, if purchased_units=2, then when operating_units=1, available capacity<=flow_cap_max * 1 (since this will be lower than flow_cap, which is flow_cap_max * 2 in this example.

@sstroemer
Copy link
Contributor Author

sstroemer commented Aug 7, 2024

@sstroemer: this issue is becoming a bit of a monster. Can you please split it to make discussion easier?

Sorry! I separated all topics besides the last clarification below. If that is working as intended then feel free to immediately close this one 😄

Yes.

Can you point me to the constraints that (combined) construct what you described?

You're missing that flow_cap >= flow_cap_min * purchased_units (flow_capacity_min_purchase_milp)

I do not see why this matters? flow_cap >= flow_cap_min is enforced in the bounds of flow_cap, and flow_cap <= flow_cap_max * purchased_units, therefore flow_cap_max * purchased_units >= flow_cap_min. If flow_cap_min > 0, then purchased_units cannot go to 0.

(yes, the RHS in flow_capacity_min_purchase_milp could go to 0, but since the bound is there, the bound would be binding instead)

@brynpickering
Copy link
Member

@sstroemer just had a look at the flow_cap_min * purchased_units issue again and you're quite right - good spot! I guess this means there needs to be a distinct constraint to set flow_cap_min, i.e. it is not set at the variable level at all. Same with storage_cap_min.

@brynpickering brynpickering linked a pull request Oct 24, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants