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

Define accessor functions #3

Merged
merged 11 commits into from
May 10, 2022
Merged

Define accessor functions #3

merged 11 commits into from
May 10, 2022

Conversation

BSnelling
Copy link
Member

Implementing accessor functions for extracting data from the System that are needed to simulate market clearing.

@codecov
Copy link

codecov bot commented May 5, 2022

Codecov Report

Merging #3 (520e848) into main (3769271) will increase coverage by 21.50%.
The diff coverage is 96.77%.

@@             Coverage Diff             @@
##             main       #3       +/-   ##
===========================================
+ Coverage   55.76%   77.27%   +21.50%     
===========================================
  Files           1        2        +1     
  Lines          52       88       +36     
===========================================
+ Hits           29       68       +39     
+ Misses         23       20        -3     
Impacted Files Coverage Δ
src/system.jl 32.14% <50.00%> (-23.63%) ⬇️
src/accessors.jl 98.33% <98.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3769271...520e848. Read the comment docs.

src/system.jl Outdated
"Returns a `Dictionary` with zonal operating reserve requirements indexed by zone number."
function get_operating_reserve_requirements(system::System)
return map(system.zones) do zone
sum([zone.reg, zone.spin])
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 don't know how this is calculated. Is it a function of the information contained in the Zone type?

Copy link
Member

Choose a reason for hiding this comment

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

If you're asking about operating reserve requirements specifically, they're just the sum of regulation and spinning reserve requirements, and are defined per zone, so I think you defined it correctly here?

Copy link
Member Author

Choose a reason for hiding this comment

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

So it doesn't include on- and off-line supplemental? After committing this I found the definition here: https://gitlab.invenia.ca/invenia/research/FullNetworkDataPrep.jl/-/blob/7773f925f5ac98aa69dac280dfa2aec6f976fb7f/src/build/reserve_requirements.jl#L18

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, I mixed things, there's another requirement that is just over reg + spin, but operating reserve is over reg + spin + supp (and the on vs off distinction doesn't make sense in this case, the docstring there is a bit confusing, since a generator cannot simultaneously provide online and offline supplemental reserve).

The problem here is that the Zone type is not defined how it should, I let this slip in a previous PR. There are no individual requirements for each service type in MISO, what they have is

  1. regulation requirement (just reg)
  2. operating reserve requirement (reg + spin + supp)
  3. good utility practice requirement (reg + spin)

so we need to redefine Zone to have these three requirements

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to check my understanding in code. The zone should have just three requirements

struct Zone
    number::Int64
    regulation::Float64
    operating_reserve::Float64
    good_utility::Float64 # we don't model this at the moment right?
end

And then the get_operating_reserve_requirements function would just be:

function get_operating_reserve_requirements(system::System)
    return map(system.zones) do zone
        zone.operating_reserve
    end
end

And when we come to model the operating reserve requirement that would involve getting time series data from all four of the ancillary services time series (asm_regulation, asm_spinning, asm_sup_on and asm_sup_off). Does this seem right?

Copy link
Member

@raphaelsaavedra raphaelsaavedra May 6, 2022

Choose a reason for hiding this comment

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

Yes, that makes sense to me.

# we don't model this at the moment right?

It seems we don't, and I'm not sure why. We do define these requirements on the data side here with a confusing name, but not on the modelling side. I'll open an issue

src/FullNetworkSystems.jl Outdated Show resolved Hide resolved
src/accessors.jl Outdated Show resolved Hide resolved
src/accessors.jl Outdated Show resolved Hide resolved
src/accessors.jl Outdated

function _get_providers(ts)
units = axiskeys(ts, 1)
providers = vec(sum(ts .!= 0.0, dims=2) .!= 0)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is doing the same as our internal code. We specifically check if a service has been added to a generator, which is different than having an offer price of zero (which would mean the generator is happy to provide the service for free).

We'd need to decide how missing entries would be listed in the KeyedArrays in GeneratorTimeSeries. They could be literally missing, and that would explicitly indicate that generator g is not providing that service in hour t, which actually creates a more flexible framework than we have today, since we assume providers are static throughout the day I think

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! I checked the ASM offers dataset and all the unit codes appear in it. I guess their offers appear as missing in that data, so would it make sense to do the same here?

Copy link
Member

@raphaelsaavedra raphaelsaavedra May 6, 2022

Choose a reason for hiding this comment

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

Yes, there are missing entries in the offer data, and it makes sense if we follow the same format here. This actually simplifies things compared to the current state

Copy link
Member Author

Choose a reason for hiding this comment

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

From an offline discussion: we concluded that having missings in the time series data was sufficient to distinguish non-providers from providers, and that using the information from the time series data when creating variables in the model should actually lead to simplifications in the model building. So for the moment we don't need the helper functions that return a list of the providers.

src/system.jl Outdated
"Returns a `Dictionary` with zonal operating reserve requirements indexed by zone number."
function get_operating_reserve_requirements(system::System)
return map(system.zones) do zone
sum([zone.reg, zone.spin])
Copy link
Member

Choose a reason for hiding this comment

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

If you're asking about operating reserve requirements specifically, they're just the sum of regulation and spinning reserve requirements, and are defined per zone, so I think you defined it correctly here?

src/accessors.jl Outdated
get_loads_per_bus(system::System) = system.loads_per_bus

"Returns the power transfer distribution factor of the system."
get_ptdf(system::System) = system.PTDF
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this PR but this made me realise that PTDF and LODF are in upper case even though they're fields, not types. Shouldn't they be in lower case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, changed to lower case.

src/accessors.jl Outdated Show resolved Hide resolved
BSnelling and others added 4 commits May 10, 2022 11:56
@BSnelling BSnelling merged commit d5ae93a into main May 10, 2022
@BSnelling BSnelling deleted the bes/accessors branch May 10, 2022 11:02
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.

2 participants