Prototype of method-forward paradigm

[daniel-thom]

This is not a changeset that would be merged into our codebase. It is just a prototype to evaluate whether we want to proceed with this approach.

The problem we're trying to solve is that many of our structs share common fields. This can be problematic when PowerSystems consumers want to extend their own types. They have to copy all of the fields. More importantly, if we ever added a new field and used that field in an exported method, those consumers would get unclear runtime errors.

One part of this solution is to move away from defining field access (value.field) as part of our public API. We would need to have accessor methods defined for every field. Instead of typing "generator.name" you would type "name(generator)".

This changeset implements the paradigm for the subtypes of RenewableGen. The common fields are in the new struct RenewableGenBase. The forward macro from ReusePatterns.jl forwards all methods to that type.

The rest of the changes were to make the tests pass or illustrate how many lines of code we will have to change.

Thanks to Dheepak for finding and proposing the "forward" macro solution.

[daniel-thom]

Sorry for the churn. I based this off my UUID/serialization branch because I wanted to show how this interacts with that work. This means that you should only look at the last commit.

[daniel-thom]

This fixes what I think is a bug. If you call RenewableFullDispatch() you actually get a value that is of type RenewableCurtailment.

[daniel-thom]

Note that this approach means that we will likely see tons of name collisions, and so will have to prefix function calls with "PowerSystems.".

[kdheepak]

I must be missing something, why do we have to prefix PowerSystems. to the name function?

[daniel-thom]

We only have to do it when name collides with a local variable, as it does here. Other possibilities are to change the local variable to something like "name_" or call the function "get_name".

[kdheepak]

Of course, my bad. I'm fine with renaming the local variable here.

[jd-lara]

Adding a core dependency on another package is less than optimal. I understand the need to have a better interface but reading the ReusePatterns developer justification for this in Discourse makes me even less certain that this is the way we should go. We had this discussion about the use of SimpleTraits.jl and arrived at the same conclusion. A search in Github reveals that no-one else is using this package as a dependency.

Also, this discussion is pretty clear about why in Julia hacked inheritance is not advisable and why these perceived issued
https://discourse.julialang.org/t/workaround-for-traditional-inheritance-features-in-object-oriented-languages/1195/17

The comments from StefanKarpinsky here are also useful https://discourse.julialang.org/t/why-is-it-impossible-to-subtype-a-struct/19876/11

Here is the issue in Julia about Abstract Types with Fields JuliaLang/julia#4935 (comment)

Also this http://www.stochasticlifestyle.com/type-dispatch-design-post-object-oriented-programming-julia/

It is worthwhile re-thinking some aspects of the way we define the fields in the structs and the type structure to make it more efficient, but I am not convinced this is the proper way.

[daniel-thom]

We could actually just copy the function macro and dependent code into PowerSystems. It isn't much code. I linked it here for simplicity. We should only focus on the use of it.

The discourse thread you linked shares the same types of discussions as the ones the ReusePatterns author linked to. They talk specifically about our problem where multiple concrete types share the same fields. Here are the main points:

• You can define an abstract type (suppose RenewableGen) and require that all subtypes implement the fields that are common. This is not very Julian. Many of the developers specifically recommend against this. It also presents lots of problems. Changing fields because of implementation changes will cause massive churn in the codebase and everyone extending it.
• A Julian solution would be to define required interfaces for RenewableGen and require that all subtypes implement that interface. That means that all subtypes would have to define methods for all common fields. That becomes tedious very quickly.
• You can automate the method generation with a forward macro. There are several packages out there that do this; some of them are written by Julia devs. From reading these threads I think that inevitably some forward macro will end up in Base. I really have no idea why that haven't already formalized it.

I think this is likely the most pragmatic solution to solve the problem. Dheepak and I spent a lot of time discussing it. I'm still very open to other options. Also, I think that this PR shows that making this change will be very painful. It's a lot of changess.

[kdheepak]

@jd-lara to add more context, let's say someone wants to use PowerSystems and extending the type we've implemented. e.g. in PowerSystems currently we have

struct RenewableCurtailment <: RenewableGen
    name::String
    available::Bool
    bus::Bus
    tech::TechRenewable
    econ::Union{EconRenewable,Nothing}
    internal::PowerSystemInternal
end

Let's say someone wants to make a CustomRenewableCurtailment that can be used interchangeably with RenewableCurtailment. If they do the following:

struct CustomRenewableCurtailment <: RenewableGen
    name::String
    available::Bool
    bus::Bus
    tech::TechRenewable
    econ::Union{EconRenewable,Nothing}
    internal::PowerSystemInternal
    custom_field::CustomType
end

and if we change RenewableCurtailment in the future, they'll have to change CustomRenewableCurtailment as well, in order to upgrade. This adds maintenance burden on developers extending PowerSystems. Alternatively, they can use composition and do this:

struct CustomRenewableCurtailment <: RenewableGen
    rc::RenewableCurtailment
    custom_field::CustomType
end

This will allow us to make changes to the fields of RenewableCurtailment in the future and developers that extend PowerSystems would get our new features when they upgrade. Also, code is not repeated in this case.

However, this is provided developers that extend PowerSystems do not use dot operator to access data. This means that in order to access the data, we need to provide a function that acts as an interface. An example for the name interface is as follows:

name(rc::RenewableCurtailment) = rc.name

A user that extends RenewableCurtailment to CustomRenewableCurtailment can also define the name function interface for CustomRenewableCurtailment. They can do this using the following:

name(crc::CustomRenewableCurtailment) = name(crc.rc)

However, it can be tedious to define all the functions for every extension. This is where having the @forward macro can be helpful. If users define the following:

@forward (CustomRenewableCurtailment, :rc), RenewableCurtailment

any function defined on RenewableCurtailment will work on CustomRenewableCurtailment, forwarding to the correct field automatically.

This is probably all obvious to you @jd-lara, I'm mainly writing it out so that others using PowerSystems in the future will know what patterns to follow for extensions, should this PR get merged and this be implemented across the package.

The only downside of this is that users of PowerSystems will not be able to use tab complete in a REPL, which I think can be remedied using (extensive) documentation.

@jd-lara do you have suggestions for alternatives that increase code-reuse and allow extensions without be tedious and putting the onus on other developers? Perhaps we can discuss this over a conference call to iron out the details.

[jd-lara]

@daniel-thom option 2 is probably the most Julian way of tackling this.

I am still unconvinced that we should support this type forwarding feature natively and if someday this capability makes into the Julia code base we can add it. I haven't seen any discussion to try to do it in the short run. Even though defining interfaces can be tedious, making a type system that can be extended with interfaces more simply is an excellent design goal if we are not there yet.

Also, it is clear we should move to a method forward use mode and rely less on the dot notation. This is a good improvement. However, I think this is unrelated to the use of macros to extended the types.

@kdheepak We have had this conversation about code re-use multiple times, and this solution all are trying to do is force Julia to work like OOP, and the discussion in the forum is long about how this just means that there is an improper type system design. The case of DataFrames is pretty well discussed.

My proposal to this "problem" is to do something similar to what is done in DifferentialEquations.jl (I have to prototype this better). Overload some functions like get_property on a custom AbstractType like this:

struct MyCustomRenewable{T<:RenewableGen} <: RenewableGen
       base_type::T
       myfield::Float64
       my_other_fied::Float64
end

t = RenewableCurtailment()
foo = MyCustomRenewable(t, 10.0, 10.0)
MyCustomRenewable{RenewableCurtailment}:
   base_type: RenewableCurtailment(name="init")
   myfield: 10.0
   my_other_fied: 10.0

In this way, a lot of methods that exist already for Renewable Energy can be extended.

Additional to this we can have a generic type for prototyping devices that is like this

struct GenericInjection <: Injection
    name::String
    bus::Bus
    available::Bool
    data::Dict{Symbol, Any}
end

and enable a type that can have any data template encoded into the Dict for users who want to add a very unconventional device.

One of the core questions here is which capabilities we want to enable such that users can extend the type structure for their use and how we want the type structure to be extended when contributing to the code. I believe most use cases so far are the former.

[claytonpbarrows]

@daniel-thom @jd-lara @kdheepak , where are we on this? If this is our path forward, I'd like to pursue it soon. If more work is required to identify the solution, I'll rearrange some priorities.

[jd-lara]

@claytonpbarrows I think we agree on principle about going on this path. The implementation is not clear yet. I have been reading about the use of the eval() function for code generation but no definitive procedure.

@daniel-thom mentioned that there are some severe performance penalties with some of the "official" approaches to solve this and we probably should document that and ask in discourse.

[daniel-thom]

Dheepak proposed an implementation in a separate repo (internal link removed). I am 100% onboard with that. I do not know of any performance penalties with it.