Get rid of SpineOpt's hidden state

[manuelma]

At the moment we call using_spinedb with the DB url to generate the SpineOpt data API in the global scope. This is so we can call unit(), node() and stuff like that from anywhere, which sounds nice but it has its drawbacks:

• We polute the global space which means more recompilation.
• It's difficult to understand where all these functions come from when you're reading the code as they are not defined anywhere (apart from the template .json file of course).
• Successive runs overwrite the global space and rumour has it sometimes it gets corrupted and you need to restart Julia.

An alternative would be to introduce a new function in SpineInterface, analogous to using_spinedb, but that instead of creating the data API in the global scope, it returns it inside a Dict. Then we could store that dict inside m.ext[:spineopt], pass the m to any function that needs access to the API (we already pass the m to most of those functions anyways, to build the model etc.) and inside those functions, @fetch (i.e. 'unpack') whatever we need in order to build a constraint or create a variable.

That's not too much work and should be straightforward.

Edit: note that apart from using_spinedb, we double down on creating globals in preprocess_data_structure which follows essentially the same logic of @evaling within a function to make stuff available to other functions - so we should also fix this.

[manuelma]

@DillonJ @abelsiqueira @suvayu any thoughts?

[DillonJ]

What about parameter calls and filter arguments... what would this dict hold? Functions or static values?

Right now we can call unit_capacity() with keyword filter arguments, a timeslice and stochastic path and the code doesn't care if it's a timeseries value, time pattern or a static value.

I'm not convinced that the "convenience functions" are a major source of complexity to be honest - I think they are one of the easier elements to understand. But maybe I'm wrong. @tarskul, what do you think ?

[tarskul]

I fully agree with Manuel, so I won't add to that, but I will answer your question directly. Anything in the global space is quite complex. For the example of unit_capacity(): It is convenient if you know it exists. But if you don't know, it is very hard to keep track of. You'll have to open up a spine db (or the template) to search for the entity (unit) and its parameter (capacity), you have to know how SpineInterface handles that data (entity_parameter()) and make a best guess of what the name of the corresponding function would be in julia code (unit_capacity()). Whereas if it would be explicitly present in a dictionary, you don't have to leave your code space and you can print the properties of 'm' and see the names written there. (There probably also is a way to print out everything in the global space but I'm not sure how and it may be a mess to go through.)

BTW: It is more than a rumor that the global space gets corrupted. I've encountered this a few times when developing the conversion scripts for the ines spec.

[manuelma]

The entire functionality will be kept @DillonJ - the only difference is we will pass the convenience functions - exactly as they are now - via the m argument - instead of making them available globally.

I think it will be more organized and welcoming for most people and also more appreciated for the computer science people.

what would this dict hold? Functions or static values?

Sorry I missed your key question... Functions of course, the convenience functions as they are now. Functions are just like any other object in julia that can be passed around and stored in dicts etc.

[manuelma]

I think the major issue with the convenience functions in the global scope is new contributors don't know they are created by calling using_spinedb somewhere early in the run_spineopt flow. They have no way of infering that from the code other than going through all of it trying to identify the place where that happens. Whereas if we just unpacked the functions from the m argument it will be much more reasonable.

[abelsiqueira]

Hi all, I don't know enough about the code to give a concrete suggestion. I can talk about the short experience I had on the day @suvayu and I looked into that one of these functions.

When we were investigating one of the indices functions that were slow. Suvayu and I saw a function or structure called connection_capacity. After a while we realized that this is constructed based on the input that you give to the SpineOpt load function, which is based - I believe - in some definition in spinedb. Based on that definition, then the code creates an ObjectClass, or RelationshipClass, or Parameter. I don't know which, because I haven't checked spinedb.

So my first questions would be

• Is the assesment correct? Is that how it works?
• What is connection_capacity?
• Why do you create connection_capacity based on data input? What information do you need dynamically?
• I searched the code via GitHub and found this call that suggests that connection_capacity is a function. If it's not a function, I have different questions and suggestions.
• Assuming it's a function, why can't you just define a function called connection_capacity? What is special about it?

[DillonJ]

Just to say @tarskul and @manuelma regarding this:

. For the example of unit_capacity(): It is convenient if you know it exists. But if you don't know, it is very hard to keep track of. You'll have to open up a spine db (or the template) to search for the entity (unit) and its parameter (capacity), you have to know how SpineInterface handles that data (entity_parameter())

My understanding is that this won't change. The convenience functions, whether global or passed as arguments, will still be named after their entity or parameter definitions in the DB - this link is fundamental, and not that complex in my opinion. I think that link between the data and the code is unavoidable

[manuelma]

My understanding is that this won't change. The convenience functions, whether global or passed as arguments, will still be named after their entity or parameter definitions in the DB - this link is fundamental, and not that complex in my opinion. I think that link between the data and the code is unavoidable

Yes, absolutely, @DillonJ - they will be called exactly as they are called now and work the exact same way, the only difference is that they will find their way into the function more explicitely. Instead of living in the global scope they will live in the model object passed as argument. That's the idea, not sure if it will make a huge difference but it will feel more standard.

[manuelma]

@abelsiqueira thank you for taking your time to check this.

connection_capacity is just a Parameter object - created when we 'load' the DB as you correctly identified. In that Parameter we store the connection capacity defined by the user in the DB for each 'entity'. Parameters are callable as functions (because we have implemented the call operator, ()) so that given the 'entity' they return the corresponding value.

The reason we do it this way is mainly to avoid all the typing. We want the API to be created automatically based entirely on the contents of the DB rather that typing all the functions in code. Every time you add something to the template you know you will have the corresponding ObjectClass, RelationshipClass or Parameter available in code.

[abelsiqueira]

avoid all the typing

Which typing? In the creation of connection_capacity? How much typing is there, i.e., how many parameters/objects/relationships are created automatically? Do you always have a connection_capacity? Is the user always required to create one? If so, is there a check?

[manuelma]

Everything from the template basically (https://github.com/spine-tools/SpineOpt.jl/blob/1b515b5c55743a0f040775b7937834c9f4f286e3/templates/spineopt_template.json) is created automatically.

The way we populate the data API is by first loading the template json file and then updating it with the contents of the user DB. So if the user is missing connection_capacity for instance (or anything else) since it is in the template it is guaranteed to exist in code - and have some default value also defined in the template.

So if we had to create a 'query' function for every item in the template we would need a lot of typing and synchronization that we want to avoid.

[suvayu]

Hi, if I'm not mistaken, values/names from the template are mostly being @evaled in SpineInterface/src/api/db.jl.

Is that right? If that's the case, then instead of using a JSON template, we can encode this information in Julia functions/structs. I don't know how difficult it will be, but it seems quite possible.

I think the compromise would slightly more verbose API; so something like connection_capacity() will become parameter_value(name::String)::Float64. WDYT?

---

My main point is: while writing a JSON template is simpler, but since the template doesn't really change frequently, it's probably worth our time, to instead encode this same information as a Julia module (which I expect to be slightly more difficult, but not impossible). This module is then essentially the API SpineOpt/users end up using.

[manuelma]

Not sure I follow fully @suvayu - one thought is we also use the JSON template so that users can load it in their DB via toolbox to start working. It's not just for the purposes of the code. The usage to initialize the SpineOpt API is secondary I guess, as a failsafe in case the user DB and the template get out of synch eventually.

[manuelma]

Let me illustrate my proposal with pseudo code to see if you guys think it's beneficial. The current situation is roughly the following:

function run_spineopt(url_in, <more args>)
    # magically create convenience functions, e.g., `connection` `connection_capacity`, etc. in the global scope
    using_spinedb(<json template>)
    using_spinedb(url_in)
    # build model
    m = Model()
    build_model!(m)
    # keep going
end

function build_model!(m)
    # Access connection and connection_capacity here from the global scope
    for conn in connection()
        flow = @variable(m, lower_bound=0)
        @constraint(m, flow <= connection_capacity(conn))
    end
end

The problem is contributors looking at the build_model! function have no way to know where connection and connection_capacity come from. My idea below (where functions_dict is analogous to using_spinedb but instead of @evaling the functions, it puts them in a Dict and returns them to the caller).

function run_spineopt(url_in, <more args>)
    m = Model()
    d = functions_dict(<json template>)
    merge!(d, functions_dict(url_in))
    m.ext[:functions] = d
    build_model!(m)
end

function build_model!(m)
    connection = m.ext[:functions][:connection]
    connection_capacity = m.ext[:functions][:connection_capacity]
    for conn in connection()
        flow = @variable(m, lower_bound=0)
        @constraint(m, flow <= connection_capacity(conn))
    end
end

[suvayu]

My quickly considered opinion is that this seems to address it.

• ~Is it expected that the user always has access to the model object?~ (Up on some thinking, this is should be the case, otherwise can't create @variables/@constraints)
• Is there any performance impact in storing arbitrary things in the model object?

[suvayu]

Btw, is the template the sole source of global functions/data structures?

[manuelma]

• it expected that the user always has access to the model object? -> Yes, typically if you want to access the convenience functions is for altering the model so yes, the model object should be available. Otherwise we could always pass it.

• Is there any performance impact in storing arbitrary things in the model object? -> I am not aware of any performance implications of using m.ext to store arbitrary data - there shouldn't be any.

• Btw, is the template the sole source of global functions/data structures? -> We also have a function called preprocess_data_structure that adds more stuff to the global - but we could easily refactor to add that stuff to m.ext[:functions] instead.

[suvayu]

This sounds like a good starting point. I guess the best way to evaluate the impact, is ask the less experienced people to review and see if that makes the code easier to follow for them.

[manuelma]

@datejada @nhniina @gnawin @tarskul @Tasqu ?

Here is the comment that attempts to summarize the proposal: https://github.com/spine-tools/SpineOpt.jl/issues/1100#issuecomment-2376972506

[manuelma]

Just a quick note to say that even though the main purpose is to improve the contributor experience, there are indisputable benefits regarding less recompilation and the warranty that no SpineOpt run will see artefacts from previous runs.

[abelsiqueira]

The proposed solution might work. It doesn't fully clear the confusion of how connection and connection_capacity were created though.

One concern with this approach is whether m.ext[:functions][:connection] can have its type inferred, and whether that will hurt performance. It might not be inferred because:

• It has many layers, which might influence how well inference works. I don't know enough to judge.
• To be general, the element type has to be abstract. For instance D = Dict(0 => 1.0, 1 => 1.0 + im) has type Dict{Int,Number}, because Number covers both Float64 and ComplexF64. Accessing D[x] is type unstable because we don't know what concrete subtype of Number will come out based solely on the type of x.

Can you check that? Can you create a standalone benchmark for that?

---

On the JSON vs explicit code topic. How often do you expect the JSON to change? I also don't understand why you have that JSON in SpineOpt instead of SpineInterface, and what exactly is drifting apart. The releases should keep things in sync even if master changes.

[manuelma]

Good points @abelsiqueira - I will try to check that inference thing but might not have time. I don't expect to have too much time to work on this anyways.

• The template is constantly changing as we add new functionality. It may stabilize at some point but I don't see that happening soon - don't know the exact road map though.
• SpineInterface is intended to serve arbitrary models, not only SpineOpt. We already have other packages that use it and have their own template. So we can't store the template in SpineInterface.
• What is drifting appart is the user DB after they have loaded the template from a specific version. If we then add more stuff to the template, that doesn't get loaded on the user DB unless they purposely do it. But to not force them to do it, we load the template from code to fill any gaps.

[manuelma]

If inference happens to be a problem what we could do is split the m.ext[:functions] into categories m.ext[:parameters], m.ext[:object_classes], m.ext[:relationship_classes] where each one is a Dict with concrete type, e.g., Dict{Symbol,Parameter}. We can even annotate them explicitely so no inference needed.

[DillonJ]

As a thought experiment - you could hard code the classes and relationship and create the functions explicitly but then the link with the Spine data structure is broken and we would have to carry out the reverse operation, create a SpineDB and template from the code rather than creating code from the SpineDB and template. The latter is more user friendly and makes it easier to add new features, but perhaps the cost of this flexibility is too high?

Say we did create the convenience functions explicitly in code... it seems to me that would be extremely tedious? Would there be a significant performance payoff? I guess it would make compiling a SpineOpt package easier - or would it?

Again, though, once you understand that the convenience functions are created from the data structure, and you know that all you need to do is look in the template... do people still feel this is the major blocker to understanding the code?

I'm thinking of @nelliputkonen's example of using AI to devine what the code was doing. I don't feel in this case it was the parameter names coming from the DB that was making the code hard to follow?

In my personal experience, these things make it difficult for me:

• Dict comprehension syntax can be hard to follow but, my understanding is that it's more performant
• Types: tuples, named tupes, arrays - figuring out the expected type of different arguments... sometimes it seems it's named tuples, sometimes lists, sometimes tuples
• Indices functions - getting your head around how to generate the indexes for a constraint, particularly the temporals and stochastics. What type is s? A path? An array of scenarios? But @manuelma's documentation should help with this
• In a lot of the code there are helper functions... like we see _unit_capacity() and stuff like that - reading the constraint code itself often isn't self-contained - but that's because of the features we have and I'm not sure if it can be done otherwise
• The @constraint macro: When a constraint generates a traceback, you don't get a specific line in the @constraint macro... and some constraints are very long with lots of terms. Not sure if something can be done about that.

[manuelma]

Hi @DillonJ excellent points. I am not convinced this is the major blocker either, this feels more like a blocker for experienced coders than for most of our contributors. Although it is the only one I recall coming up in the meeting with sufficient detail - but yes, people should post their own blockers and eventually create issues in my opinion so we can tackle them one by one.