PowerSimulations.jl icon indicating copy to clipboard operation
PowerSimulations.jl copied to clipboard

Jd/add thermal time series

Open jd-lara opened this issue 1 year ago • 5 comments

closes #1153 partially. Still missing situations when this is mixed with dispatch models.

If the approach in this PR is ok, I will implement for dispatch models and two stage simulations.

jd-lara avatar Oct 13 '24 20:10 jd-lara

Performance Results

Version Precompile Time
Main 4.394310773
This Branch 4.489718702
Version Build Time
Main-Build Time Precompile 67.457353893
Main-Build Time Postcompile 1.014952664
This Branch-Build Time Precompile 69.518155119
This Branch-Build Time Postcompile 1.095375151
Version Build Time
Main-Solve Time Precompile 1195.303225806
Main-Solve Time Postcompile 1160.332067219
This Branch-Solve Time Precompile 873.019986163
This Branch-Solve Time Postcompile 835.260607043

github-actions[bot] avatar Oct 13 '24 21:10 github-actions[bot]

Codecov Report

Attention: Patch coverage is 57.14286% with 27 lines in your changes missing coverage. Please review.

Project coverage is 76.30%. Comparing base (5863e9e) to head (fc6fc96).

Files with missing lines Patch % Lines
...vice_constructors/thermalgeneration_constructor.jl 55.00% 18 Missing :warning:
.../devices_models/devices/common/range_constraint.jl 33.33% 8 Missing :warning:
...rc/parameters/update_container_parameter_values.jl 50.00% 1 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1162      +/-   ##
==========================================
- Coverage   76.40%   76.30%   -0.10%     
==========================================
  Files         121      121              
  Lines       13089    13149      +60     
==========================================
+ Hits        10000    10034      +34     
- Misses       3089     3115      +26     
Flag Coverage Δ
unittests 76.30% <57.14%> (-0.10%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/PowerSimulations.jl 100.00% <ø> (ø)
...evices_models/devices/default_interface_methods.jl 35.71% <ø> (ø)
src/devices_models/devices/thermal_generation.jl 84.71% <100.00%> (+0.11%) :arrow_up:
src/parameters/add_parameters.jl 74.20% <100.00%> (+0.47%) :arrow_up:
src/services_models/services_constructor.jl 97.60% <100.00%> (ø)
...rc/parameters/update_container_parameter_values.jl 34.90% <50.00%> (+0.14%) :arrow_up:
.../devices_models/devices/common/range_constraint.jl 73.06% <33.33%> (-1.65%) :arrow_down:
...vice_constructors/thermalgeneration_constructor.jl 94.49% <55.00%> (-3.82%) :arrow_down:

codecov[bot] avatar Oct 13 '24 22:10 codecov[bot]

@rodrigomha and @sourabhdalvi take a look at the branch. I think this can benefit from more testing but this approach should suffice.

@daniel-thom Can you confirm the use of has_time_series is adequate in this context? If this approach works we can implement for other situations where not all the devices might have time series and save the users the need to define vectors of 1.0's

jd-lara avatar Oct 13 '24 23:10 jd-lara

@rodrigomha and @sourabhdalvi take a look at the branch. I think this can benefit from more testing but this approach should suffice.

@daniel-thom Can you confirm the use of has_time_series is adequate in this context? If this approach works we can implement for other situations where not all the devices might have time series and save the users the need to define vectors of 1.0's

I expect that this is substantially slower than the PSY 3.0 version because each call to has_time_series corresponds to one SQL query. It's probably the difference between nanoseconds and microseconds/low-milliseconds. If that is concerning, we could make a new function to get components with time series that makes a single SQL query. It could work with either get_components or an existing iterable of components.

This depends on how many times the function will be called (how many components).

julia> length(get_components(Component, sys))
525

julia> @time collect(get_components(x -> has_time_series(x), Component, sys));
  0.271341 seconds (95.72 k allocations: 6.984 MiB, 74.16% compilation time)

julia> @time has_time_series(g)
  0.002146 seconds (31 allocations: 1.922 KiB)

daniel-thom avatar Oct 14 '24 16:10 daniel-thom

@rodrigomha and @sourabhdalvi take a look at the branch. I think this can benefit from more testing but this approach should suffice. @daniel-thom Can you confirm the use of has_time_series is adequate in this context? If this approach works we can implement for other situations where not all the devices might have time series and save the users the need to define vectors of 1.0's

I expect that this is substantially slower than the PSY 3.0 version because each call to has_time_series corresponds to one SQL query. It's probably the difference between nanoseconds and microseconds/low-milliseconds. If that is concerning, we could make a new function to get components with time series that makes a single SQL query. It could work with either get_components or an existing iterable of components.

This depends on how many times the function will be called (how many components).

julia> length(get_components(Component, sys))
525

julia> @time collect(get_components(x -> has_time_series(x), Component, sys));
  0.271341 seconds (95.72 k allocations: 6.984 MiB, 74.16% compilation time)

julia> @time has_time_series(g)
  0.002146 seconds (31 allocations: 1.922 KiB)

We need to figure this out because this is critical for NTP and other systems and we can't have this kind of performance hit since this is called multiple times in multiple places. We ought to put some other way to compute this property.

jd-lara avatar Oct 14 '24 16:10 jd-lara