[trulsf]

This is an attempt at starting work on modeling investment relations and couplings. If this is interesting to include, there will be a need for a couple of following PRs on documentation and better integration with EnergyModelsBase by modifying EMIExt so that user-facing functionality is a bit more intuitive.

I have prioritized relations that are not dependent upon binary variables (as they have more general usage), but it is possible to add more relations that depend upon binary variables (e.g. having a requires() independent of capacity ratios). There are a lot more possibilities and flexibility that could be added (e.g. lead times), but they can be easily added if we agree on the overall value and design of such an extension.

Tests are mostly AI written, so they should be properly checked.

[JulStraus]

I like the general concept. I have to take a closer look at it and will come back to you with a more detailed feedback.

I will create a new commit in which I do some general adjustments regarding the style while I will look through it and commit this before I write my direct comments on the code.

[JulStraus]

I did a few EMX approach adjustments in the individual commits. These are mostly style related as well as an improvement on the docstrings.

I overall like the concept. It would require a specific API for simplified implantation for anything from EMX, but these are the not too difficult to implement. I would like to highlight the following things which should be discussed:

1. max_investments, min_investments, and excludes_capacity are based on investment actions. This implies that excludes_capacity does not enforce that a capacity is retired if it is already existing before another technology capacity is added. I think @hellemo also would like the latter. It could be however easily added, if desired.
2. The individual limits or ratios are always numbers. This makes sense if we want to some it for all periods, but could be wrong if we do want different values in the different periods. As an example, I would allow capacity_ratio in requires_capacity, couple_capacity, and precede_capacity to be a TimeProfile.
3. max_budget is the undiscounted budget. Should it go towards the discounted budget? If so, it would require a discounting rate.
4. There are also other potential varieties which we could include. As an example, we could also base some of the investments on additional binary variables or alternatively disjunctions.

There are a few things which must be adjusted. The following changes should be fixed within this PR:

1. precede_capacity is in my opinion wrong due to the summation, see the specific comment.
2. Improvement on the tests.
   The current tests are in my opinion not good, as they pretty much focus only on the termination status.
3. Update of NEWS.md (Under the header unversioned, if we decide to have some parts in a different PR)
4. Documentation:
   The documentation requires more information in the library as I currently only reference the docstrings in the public library.

In another PR (or this), we should fix the documentation. It requires a description of the constraints in math_desc.md before the section Utility functions called Constraints for investment relationships and in a section below Lifetime modes called Investment relationships. The latter explains the core concept.

I could fix the second PR regarding the documentation.

[trulsf]

Some more updates from my side. precedes_capacity has an adjusted interpretation. Tests have been revised. Two more relations have been added. I have no strong opinion on whether capacity should be time ratios or not (although I am bit uncertain about a good use case). I leave it to @hellemo to decide upon whether discounted budget limits make sense.

[JulStraus]

I reworked the testsets now and renamed excludes_capacity to exclude_investment. it seems to me more precise as it is an exclusion on investments and not installed capacity. I have for the latter an approach local which requires new binaries, but I am uncertain if I want to include it as well here. This PR is already way bigger than I had hoped for...

We could split it up into 3 based on the testset structure, but I am not certain that it would be beneficial, mostly as all the concepts are individual with new code. It is hence also ok to leave it like that. I would however like to have one page in the documentation (separate PR, I can do it). Aside from that, it looks good to me now

[JulStraus]

With my recent changes I am happy with it. What is your take @hellemo ?

[hellemo]

I think this is coming along nicely, thanks!

I have some questions/comments to the API, which is not entirely consistent, and we could perhaps simplify a bit with default values for the prefixes that are typically used.

Several functions use a Vector{<:Tuple{Symbol, <:Any}} for investments, could we provide a method with a default value :cap for all investments for convenience?

Other functions have split the arguments into single elements and prefix (prefix_dep and prefix_pre). Could we either rearrange those and set default value for the prefixes or alternatively rethink the interface to use a Vector of Tuples here as well? (Vectors may not make sense for all, but at least multiple elements might depend on the same element, or exclusivity could be useful for a vector as well.

e.g.:

    exclude_investment(
        m,
        element_1,
        element_2,
        𝒯::Union{TwoLevel, TwoLevelTree},
        prefix_1::Symbol = :cap,
        prefix_2::Symbol = :cap,
    )

I also considered (but did not conclude) whether it would be nice to allow to specify the actual strategic periods and default to the current behaviour, i.e. 𝒯ᴵⁿᵛ = strategic_periods(𝒯), which would allow to limit the scope of the constraints if desired and save a function call if you have the periods readily available.

something like:

    exclude_investment(
        m,
        element_1,
        element_2,
        𝒯::Union{TwoLevel, TwoLevelTree},
        prefix_1::Symbol = :cap,
        prefix_2::Symbol = :cap,
        𝒯_inv = strategic_periods(𝒯),
    )


Setting limits on discounted vs nominal values was raised, I think nominal values would be easiest to understand and use, not sure what would be the use case for specifying discounted values.

[JulStraus]

I have some questions/comments to the API, which is not entirely consistent, and we could perhaps simplify a bit with default values for the prefixes that are typically used.

I do not think that is reasonable for EnergyModelsInvestments as we do not create any variables within the package. However, this is something which should be included for the API for the extension in EnergyModelsBase as we can automatically deduce the respective prefix from the subtype of AbstractElement. As an example, it will always be :cap for any standards nodes and :trans_camp for all TransmissionModes. Storage nodes are in this case a bit more complex.

Several functions use a Vector{<:Tuple{Symbol, <:Any}} for investments, could we provide a method with a default value :cap for all investments for convenience?

Other functions have split the arguments into single elements and prefix (prefix_dep and prefix_pre). Could we either rearrange those and set default value for the prefixes or alternatively rethink the interface to use a Vector of Tuples here as well? (Vectors may not make sense for all, but at least multiple elements might depend on the same element, or exclusivity could be useful for a vector as well.

While I agree that it is not really intuitive, I do not think it is easily possible to utilize a common format. The reason why we have both Tuples and specific inputs is that in the cases of Tuples, we do not care too much how many elements are coupled. In the cases we have specific input arguments, it is important to have an understanding which technology is the prerequisite and which is the dependent. I cannot find something easily right now to make it flexible and consistent for both cases....

I also considered (but did not conclude) whether it would be nice to allow to specify the actual strategic periods and default to the current behaviour, i.e. 𝒯ᴵⁿᵛ = strategic_periods(𝒯), which would allow to limit the scope of the constraints if desired and save a function call if you have the periods readily available.

I think we should provide the option for that. Your suggestion could be a solution or alternative we could use the same approach as max_investments, min_investments, and max_budget.

Setting limits on discounted vs nominal values was raised, I think nominal values would be easiest to understand and use, not sure what would be the use case for specifying discounted values.

Then we should leave it that way :)

[JulStraus]

@trulsf and @hellemo:
I updated the API to be unified, added a documentation, and restructured the test sets to be simpler to maintain in a latter stage. Let me know what you think. Minor changes in the API, e.g., argument ordering or move the tuple again to the previous version could be easily incorporated.

Once we agree on that it is a sufficiently good state, I will do a last commit for the version number update, merge the PR, and register.

[trulsf]

Looks good to me. As a note on further improvements (at a later stage, not as part of this PR) - I would be happy if the offered functionality was symmetrical for capacity and investments. This includes e.g. exclude_capacity (no simultaneous capacity existing for two techs at the same time - would require some extra binary variables, I guess) and precede_investment (which probably is superfluous).

[hellemo]

The API looks much better now, thanks.

One thing I missed on the previous round was that I think we typically want to be able to model exclusivity over the full set of strategic periods, not per strategic period.