Replace parameterized type variables #1012
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
daniel-thom
commented
Sep 21, 2023
| get_available_components(T, sys, get_attribute(model, "filter_function")) | ||
| add_parameters!(container, ActivePowerTimeSeriesParameter, devices, model) | ||
| get_available_components(T, sys, get_attribute(device_model, "filter_function")) | ||
| add_parameters!(container, ActivePowerTimeSeriesParameter, devices, device_model) |
There was a problem hiding this comment.
This was an existing bug because model was not defined. Please confirm the fix.
jd-lara
requested changes
Sep 21, 2023
src/devices_models/device_constructors/hvdcsystems_constructor.jl
Outdated
Show resolved
Hide resolved
daniel-thom
commented
Sep 21, 2023
| D <: AbstractThermalDispatchFormulation, | ||
| S <: PM.AbstractActivePowerModel, | ||
| } | ||
| model::DeviceModel{T, <:AbstractThermalDispatchFormulation}, |
There was a problem hiding this comment.
Changes to DeviceModel were not required. However, since these type variables were not used, I removed them.
daniel-thom
commented
Sep 21, 2023
| container, | ||
| devices, | ||
| device_model, | ||
| typeof(network_model).parameters[1], |
There was a problem hiding this comment.
This was an existing bug because S was not defined. Please confirm the change.
There was a problem hiding this comment.
This is correct. We probably should extend the tests to cover this area of the code
jd-lara
requested changes
Sep 21, 2023
| D <: AbstractThermalDispatchFormulation, | ||
| S <: PM.AbstractActivePowerModel, | ||
| } | ||
| model::DeviceModel{T, <:AbstractThermalDispatchFormulation}, |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| container, | ||
| devices, | ||
| device_model, | ||
| get_network_formulation(network_model) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| get_network_formulation(network_model) | |
| get_network_formulation(network_model), |
daniel-thom
force-pushed
the
dt/remove-unused-variables
branch
from
e7b38ae to
4b23a31
Compare
This change allows SnoopCompile.jl to generate precompile statements. For a reason unknown as of now, SnoopCompile.jl generates invalid statements for PowerModels.AbstractPowerModel and PowerModels.AbstractActivePowerModel when type variables are used.
daniel-thom
force-pushed
the
dt/remove-unused-variables
branch
from
4b23a31 to
0eacd45
Compare
|
Performance Results
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1012 +/- ##
==========================================
- Coverage 79.94% 79.94% -0.01%
==========================================
Files 116 116
Lines 12338 12337 -1
==========================================
- Hits 9864 9863 -1
Misses 2474 2474
Flags with carried forward coverage won't be shown. Click here to find out more.
|
jd-lara
approved these changes
Sep 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR allows SnoopCompile.jl to generate precompile statements for PowerSimulations.jl. In many cases I removed unused parameterized type variables. In other cases they were being used and I replaced uses with
typeof(x). There should not be any functional or performance difference.If this change is approved, I can move forward with integrating SnoopCompile.jl with this package in a separate branch. The only other realistic alternative is to report an issue with SnoopCompile.jl and see if they can help.