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

Fix Bugs/Issues of PowerFlow #45

Merged
merged 8 commits into from
Aug 12, 2024
Merged

Fix Bugs/Issues of PowerFlow #45

merged 8 commits into from
Aug 12, 2024

Conversation

rodrigomha
Copy link
Collaborator

Closes #44 #43 #40 #41

@rodrigomha rodrigomha requested review from jd-lara and GabrielKS August 6, 2024 21:04
@rodrigomha
Copy link
Collaborator Author

@GabrielKS @jd-lara The bug in the Q redistribution was that the units was hitting limits in iterative process of the remaining Q, but it was not correctly substracted to the residual in that case. This was fixed by replacing reallocated_q += q_frac by reallocated_q += q_set_point - current_q and properly checking the condition of the limits by using q_frac + current_q rather than the clamped value.

@rodrigomha
Copy link
Collaborator Author

Regarding the tests: Tests are failing for the net_flows with the update of #41.

@GabrielKS @jd-lara Let me know if we are OK with adding the symmetrical and I will update the tests to change the zeros for negative values if needed, or going back to only having one direction and the other zero.

@GabrielKS
Copy link
Contributor

GabrielKS commented Aug 6, 2024

Regarding the tests: Tests are failing for the net_flows with the update of #41.

@GabrielKS @jd-lara Let me know if we are OK with adding the symmetrical and I will update the tests to change the zeros for negative values if needed, or going back to only having one direction and the other zero.

I like the new behavior you have implemented.

Copy link
Contributor

@GabrielKS GabrielKS left a comment

Choose a reason for hiding this comment

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

Looks good aside from the test failures.

test/test_dc_powerflow.jl Show resolved Hide resolved
src/post_processing.jl Outdated Show resolved Hide resolved
@rodrigomha
Copy link
Collaborator Author

@GabrielKS It is ready for review.

There is now one kwarg when calling solve_ac_powerflow!: max_redistribution_iterations that controls the max number of redistribution iterations

@GabrielKS GabrielKS self-requested a review August 12, 2024 17:48
Copy link
Contributor

@GabrielKS GabrielKS left a comment

Choose a reason for hiding this comment

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

LGTM

@rodrigomha rodrigomha merged commit 035a207 into main Aug 12, 2024
6 checks passed
@GabrielKS GabrielKS deleted the rh/update_bugs branch November 5, 2024 22:03
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.

Bug in the weeds of _reactive_power_redistribution_pv
2 participants