search

EnergyModelsX / EnergyModelsBase.jl

MIT License
8 stars 0 forks source link

Add support for EnergyModelsInvestments as extension #34

Closed hellemo closed 2 months ago

hellemo commented 5 months ago

This is related to ongoing work to make EnergyModelsInvestments (EMI) independent of EnergyModelsBase (EMB). This is a first working draft to discuss. This PR makes the following changes to EMB:

  1. Relax type constraints on modeltype to not force extensions to depend on EMB
  2. Relax type constraints on data to not force extensions to depend on EMB
  3. Move EMB specific code from EMI to a new extension of EMB (EMIExt)
  4. Format code using JuliaFormatter

Things to do:

JulStraus commented 5 months ago

I moved the EMB specific types to the EMB extensions. This includes as well all legacy constructors, except for TransInvData.

This has two major implications. Some functions we used previously in the GeoExt are now required to be defined in the new extension in EnergyModelsGeography, namely EMI.investment_data(tm::EMG.TransmissionMode) , has_investment(tm::EMG.TransmissionMode), and EMI.start_cap(tm::EMG.TransmissionMode, t_inv, inv_data::NoStartInvData, cap).

The main reason to redefine them is to allow reference methods in EnergyModelsInvestments that provide an error if the function is not specified directly by the user.

Thinks to be fixed in this PR:

  1. Add again the EnergyModel dependency for all functions. There is no reason to not have it. Any potential problem should be solved the same way as it is solved within the investment extension.
  2. Add again the dependency of the data as data::Vector{Data} for the nodes. Again, we do not want the user to provide anything that is not a proper EMB.Data as that may result in unforeseen problems.
  3. Check all links in the documentation to point towards the proper directory in EMI. I used a lot of "@ref" for a simplified cross referencing in the data types. I would say this is something we do entirely at the end.

I also want to reverse in the end some formatting from JuliaFormatter. In my opinion, constraint macro formatting is awful with JuliaFormatter as it does not capture the intrinsics of the mathematical equations.

hellemo commented 5 months ago

@JulStraus I added some options to JuliaFormatter to keep more of the manual formatting. Check model.jl and constraint_functions.jl for some examples where constraints are formatted manually before formatting with JuliaFormatter. I think this will satisfy most if not all concerns when it comes to formatting of constraints.

JulStraus commented 4 months ago

@JulStraus I added some options to JuliaFormatter to keep more of the manual formatting. Check model.jl and constraint_functions.jl for some examples where constraints are formatted manually before formatting with JuliaFormatter. I think this will satisfy most if not all concerns when it comes to formatting of constraints.

Generally speaking, it looks better now. An alternative could also to use ignore for constraint_functions.jl and data_functions.jl, although I am not fully certain that it would improve it.

codecov-commenter commented 2 months ago

Welcome to Codecov :tada:

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

:information_source: You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered :open_umbrella: