Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: subtype System to enable modelling alternative market clearing formulations #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BSnelling
Copy link
Member

Alternative to #33 for enabling modelling different markets. The required fields needed in the System struct are fairly different between the two markets. For example the new market has load services, which don't exist in the first market. Also energy bids (both offers and demand bids) need to contain extra time series information (hence the new type EnergyBids).

Open question, both markets have the concept of GeneratorTimeSeries but the requirements might be different enough that this should be subtyped too.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #34 (05e4d61) into main (2733354) will decrease coverage by 0.72%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main      #34      +/-   ##
==========================================
- Coverage   85.89%   85.16%   -0.73%     
==========================================
  Files           5        5              
  Lines         234      236       +2     
==========================================
  Hits          201      201              
- Misses         33       35       +2     
Impacted Files Coverage Δ
src/system.jl 47.91% <0.00%> (-2.09%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

"Generator maximum output in the ancillary services market (pu)"
regulation_max::KeyedArray{Float64, 2}
regulation_max::Union{Missing, KeyedArray{Float64, 2}}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regulation_min and regulation_max do not exist as parameters for modelling the new market so these fields can be missing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we intend to reuse code, is it easier to set the values to something like Inf rather than checking for missings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting question, this is how regulation_max is used in FNModels. I think if we set it to Inf and tried to define that constraint, that wouldn't give us the intended behaviour?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure but it looks like it would work out to a constraint that is always true? We might have to reformat the expression or add an extra constraint but it might be worth it to avoid having to special-case things to account for possible missings.

@@ -249,11 +262,11 @@ Base.@kwdef struct GeneratorTimeSeries
"Generation of the generator at the start of the time period (pu)"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type may also need additional fields for multi_hour time series data. The offer curves don't appear to have multi hour flags but the ancillary services do. If the distribution of multi hours flags for a given day and resource can be different for the different services, we would need four multi hour time series, one for each of the ancillary services.

Bool indicating whether the bid is a fixed type which can only clear at the given price
and volume.
"""
is_fixed_type::KeyedArray{Bool, 2}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a Boolean going to be sufficient here? There are three types of bid: curve, variable and fixed. In terms of how the formulation is implemented I think variable is just a curve with only one block. In which case we just have to distinguish which ones are fixed.

off_supplemental_offers::KeyedArray{Union{Missing, PriceSensitiveBid}, 2}
end

struct EnergyBids
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a good name? When we refer to "bids" it can mean bids in either direction. But in the market data "bid" means demand and "offer" means supply.

"Energy offers time series data. `KeyedArray` where the axis keys are `bid ids x datetimes`"
energy_offers::EnergyBids
"Energy demands time series data. `KeyedArray` where the axis keys are `bid ids x datetimes`"
energy_demands::EnergyBids
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these field names make sense? Previously we have referred to increment and decrement bids but these EnergyBids are not necessarily virtual, so do "energy_offers" and "energy_demands" make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the names make sense but since we're already explaining things in terms of "supply" and "demand" to clarify, perhaps those are the more straightforward terms to use? Or will all markets use these terms consistently?

@raphaelsaavedra
Copy link
Member

I like the idea of subtyping system because ISO-specific systems seem to be significantly different. The main problem is that this doesn't just apply to systems, as evidenced by

regulation_min and regulation_max do not exist as parameters for modelling the new market so these fields can be missing.

So my feeling is that we need to gauge if subtyping just system is good enough, or if we need to parametrise everything by ISO – e.g., we could have this package depend on ElectricityMarkets (which would make this not open source anymore...), and then have types for each ISO following a structured type hierarchy so that we can easily dispatch things on System down the line.

Essentially, it seems to be that the main challenges stem from us trying to reconcile ISOs being very different with the fact that we want to have them all structured under a single, general framework. It might be the case that the best thing is for each one to have its own structures.

@BSnelling
Copy link
Member Author

we need to gauge if subtyping just system is good enough, or if we need to parametrise everything by ISO

From what I can see at the moment the time series requirements between the two ISOs are pretty different. To accommodate both will require subtyping System and GeneratorTimeSeries.

So far we only know about 1.5 ISOs, but it seems like the static components are consistent (Generator, Branch, Bus) and I assume the calculation of PTDF and LODFs will be similar too? If that's the case those types and that functionality are worth keeping here as part of a shared framework.

@raphaelsaavedra
Copy link
Member

seems like the static components are consistent (Generator, Branch, Bus) and I assume the calculation of PTDF and LODFs will be similar too? If that's the case those types and that functionality are worth keeping here as part of a shared framework.

I have a feeling that will be the case (system and time series are different, static components are the same / very similar)

Fields:
$TYPEDFIELDS
"""
Base.@kwdef mutable struct FinancialSystemDA <: System
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this name since it clashes with another widely-used susbystem in our codebase that we are probably going to have to deal with eventually. Is there another appropriate name?

"Generator maximum output in the ancillary services market (pu)"
regulation_max::KeyedArray{Float64, 2}
regulation_max::Union{Missing, KeyedArray{Float64, 2}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we intend to reuse code, is it easier to set the values to something like Inf rather than checking for missings?

"`Dictionary` where the keys are bus names and the values are generator ids at that bus"
gens_per_bus::Dictionary{BusName, Vector{String}}
"`Dictionary` where the keys are bus names and the values are increment bid ids at that bus"
energy_offers_per_bus::Dictionary{BusName, Vector{BidName}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these similar to incs_per_bus in the other systems? Should we try to keep the names consistent?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think they are identical in terms of type. It's a question of different concepts. incs/increments refer specifically to virtual bids, whereas here the energy_offers can be physical or virtual and we don't have a way to distinguish between the two.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should go with the most generic option then use a type to differentiate if we need to? If we use them similarly and they behave in a similar way that is.
It seems a shame to have duplicate code when we could use the same accessor across systems,

load_services::LoadServices

"Energy offers time series data. `KeyedArray` where the axis keys are `bid ids x datetimes`"
energy_offers::EnergyBids
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we switch the other systems to use EnergyBids as well? It looks like we should be able to infer the extra information required

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that could work. I suppose there would be some (minimal?) overhead to storing the extra information, but the advantages of reusable code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's repeated information and is generally read-only we could probably use an efficient data structure to avoid too much extra storage

@@ -1,6 +1,7 @@
const MARKET_WIDE_ZONE = -9999
const BidName = InlineString31
const ZoneNum = Int64
const PriceSensitiveBid = Vector{Tuple{Float64, Float64}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably document what the tuple members represent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants