[luke-kiernan]

Fixes #381.

• FixedOutput renewables: scale the reactive parameter multiplier by tan(acos(pf)).
• RenewableConstantPowerFactor: q = p · tan(acos(pf)), not sin.

Thermal/hydro FixedOutput have the same Q = P default but no power_factor field, so they are left for a follow-up.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The focused fixes match the reported defects and include targeted regression coverage; tests were not executed.

No open issues 🎉

What changed in this PR

Corrects renewable reactive-power calculations in AC models, addressing issue #381. Thermal and hydro behavior remains unchanged.

Changes:

• Scales renewable FixedOutput reactive multipliers by tan(acos(pf)).
• Corrects the constant-power-factor equation and clarifies its ratio variable.
• Adds regression tests for both corrections at power factor 0.8.

File	Description
test/​test_device_renewable_generation_constructors.jl	Tests reactive multipliers and constraint coefficients.
src/​static_injector_models/​renewable_generation.jl	Corrects both renewable reactive-power calculations.

_{Review effort: Balanced}

[github-actions]

Performance Results

Version	Precompile Time
Main	2.649021889
This Branch	2.537325029

Version	Build Time
Main-Build Time Precompile	79.838039774
Main-Build Time Postcompile	1.065620671
This Branch-Build Time Precompile	77.800950962
This Branch-Build Time Postcompile	0.980543114

Version	Solve Time
Main-Solve Time Precompile	2060.804242517
Main-Solve Time Postcompile	2046.05821947
This Branch-Solve Time Precompile	1730.598133745
This Branch-Solve Time Postcompile	1702.038840577

[luke-kiernan]

Reverted to draft momentarily while I figure out how to make all 3 of the below happen at once:

1. When there's no user-provided reactive power time series, then we use Q = P * tan(acos(pf)).
2. When there is a user-provided reactive power time series, we use that one.
3. In both cases above, the power flow in the loop sees the same value as optimization problem.

edit: oh, a further oddity. If you have a reactive power time series that's attached to some devices but not all, then the ones with the time series get treatment 2 above, but the rest get Q = 0, when you'd expect them to get treatment 1. This just seems like a side effect of device model being specified per component type, though.

edit: back to ready for review. I decided just to open a stacked PR

[rodrigomha]

Reviewed and ran it merged onto current main: the renewable, load and power-flow-in-the-loop test files all pass (1749/1749). The red CI is the FD-vs-NR tolerance failure that #412 fixed, so merging main should turn it green.

One request: guard against `power_factor == 0`.
PSY accepts `power_factor` in [0, 1], and `tan(acos(0)) ≈ 1.6e16`.
A renewable with pf = 0 would get a reactive multiplier (`FixedOutput`) or constraint coefficient (`RenewableConstantPowerFactor`) of about 1e16 times its active power, and nothing would say so.
The old `sin` version at least capped the ratio at 1.
Could both paths throw an `IS.ConflictingInputsError` at build time when pf is 0, naming the device?

[rodrigomha]

See above about pf = 0

[rodrigomha]

Approving. The pf = 0 guard covers both the FixedOutput multiplier and RenewableConstantPowerFactor, with a test for each. At f5dbee8, the renewable, load and power-flow-in-the-loop test files pass locally (1751/1751), and the formatter is clean.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[luke-kiernan]

This is finished and good to merge from my perspective--just I don't have permissions to do so