Standardize Importing Conventions From IS

[GabrielKS]

Mostly not an issue with the IS codebase itself, but an issue with how all the other Sienna packages use IS. Since IS doesn't export anything, it is up to things that import it what identifiers they want to explicitly bring into their own namespace, and we are pretty inconsistent about this.

Sometimes, it has been argued, we might define a function in a downstream package that has the same name as a semantically similar function in IS because we only want the downstream function to be visible to the user, e.g., when they query for documentation — this is the reasoning for get_components.

This gets more complicated when there are multiple downstream packages that both define their own versions of a function rather than adding methods to the IS version. Here's a case where there are four different functions that are semantically all the same thing:

PSI isn't importing IS's version, but also PSI isn't importing PSY's version, so things get complicated.

That gets even worse when multiple downstream packages export two semantically identical functions with the same name:

When thinking through how to import things from IS.Optimization into PowerSimulations, I made a list of categories starting here, but I ended up missing a few functions, causing https://github.com/NREL-Sienna/PowerSimulations.jl/issues/1125.

My hope is that at some point we can come up with some guidelines that rigorously define how things ought to be imported across the Sienna system and document them somewhere to avoid future issues like this.

[GabrielKS]

The fact that IS.get_components !== PSY.get_components has made the ComponentSelector (https://github.com/NREL-Sienna/InfrastructureSystems.jl/pull/342, https://github.com/NREL-Sienna/PowerSystems.jl/pull/1197) implementation less clean (see references here and here). The larger theme here — that it would be nice if system-like structs had a shared interface that included the same get_components function — in my opinion outweighs the concerns about confusing the user with extra methods.

[kdayday]

This is also an issue for documentation consistency. I have just found that the IS version of get_components describes the filter_func argument, and the PSY version does not have an argument list, only selected examples -- so the filter function wasn't clearly stated as an option in PSY.

[GabrielKS]

Just ran into another problem with this. I want to implement get_components on sets of results that forwards to the attached system, filtering to only available components when relevant. If get_components, get_available_components, etc. were the same function across IS, PSY, and PSI, the implementation plan would be straightforward:

1. In IS, implement get_available_components that forwards to get_components for all system-like structs that have a get_components
2. In PSY, implement get_available_components on System that filters get_components by get_available (this is already partially done)
3. In IS, implement get_components on OptimizationProblemResults that forwards to get_available_components of get_source_data
4. In PSI, implement get_components on SimulationProblemResults that forwards to get_available_components of get_system

In the status quo, this is far more complicated: if OptimizationProblemResults.source_data is an IS type, we need to use IS.get_components, but if it's a PSY type, we need to use PSY.get_components. I can't think of a non-hacky way to do this if those are not the same function. Maybe the least worst option is to only define get_components(..., ::OptimizationProblemResults) in PSY or PSI and only have it work for PSY type source_data, which is inflexible and (adjacent to?) type piracy.

[GabrielKS]

I am now officially in support of making PSY.get_components === IS.get_components and the same for all related functions (get_component, get_available_components, etc.); I'd be happy to do the implementation of this.

[kdayday]

@GabrielKS , relatedly, I am pushing documentation updates for get_components and get_available_components in PSY -- would be great if those could be incorporated wherever you do this merge. NREL-Sienna/PowerSystems.jl#1206