Pvlib-python: PVSystem.pvwatts_ac uses module capacity instead of inverter capacity

Created on 5 Jun 2019  路  16Comments  路  Source: pvlib/pvlib-python

Describe the bug
The pvwatts_ac function in the PVSystem class uses 'pdc0' from the module parameters. This is the direct current capacity of the module. According to the paper, the value that should be used is the direct current capacity of the inverter.

Versions:

  • pvlib.__version__: 0.6.3

Additional context
See page 11 of https://pvwatts.nrel.gov/downloads/pvwattsv5.pdf

Can someone confirm this?
This leads to bugs using pvwatts in ModelChain as well.

bug

Most helpful comment

Yes, thank you @janinefreeman. @JPalakapillyKWH please proceed with the PR, and THANKS!. I suggest we expand the docstring for pvwatts_ac to better describe each input.

All 16 comments

The implementation of PVSystem.pvwatts_ac is expecting pdc0 to be found in the module_parameters dict, which isn't where the parameter _should_ be found - it should be in inverter_parameters. There is no module parameter named pdc0 for any of the module models.

I think the bug is passing module_parameters['pdc0'] rather than inverter_parameters['pdc0'] which should be populated by pvsystem.retrieve_sam

Right, exactly. I'll submit a PR.

I'm curious why the tests passed.

I'll have to make changes in the modelChain code too, since the checks for the pvwatts_ac model are for the module_parameters.

No idea about the tests.

@JPalakapillyKWH on second thought, please hold off on the PR until we can get clarity from NREL. The PVWatts documents uses the symbol Pdc0 in the equation for DC power (Eq. 8) and for the inverter efficiency (Eq. 10). It's not clear from the description if the symbol refers to the same quantity in both equations, or is a different quantity. @janinefreeman can you help here?

I think they used Pdc0 for nameplate DC capacity in both Eq. 8 and Eq. 10.
But in Eq. 8 it is the nameplate capacity of the module/array while in Eq. 10 it is the nameplate capacity of the inverter.

In Eq. 11, you can see that the system is clipped to Pac0 (inverter limit derived from Pdc0) if the produced power is greater.

In the technical manual, the same symbol (Pdc0) is used for both the nameplate DC capacity of the modules in Eq 8 and the inverter DC capacity in Eq 10, but they represent two different quantities as @JPalakapillyKWH surmises (confusing, I agree). However, in the PVWatts code itself, pdc0 refers to the inverter DC capacity. System dc capacity is called "dc_nameplate" in the pvwattsv5 module (Eq 8 is on line 327 in pvwattsv5), which is assigned from the input "system_capacity".

Hope that helps clarify!

Yes, thank you @janinefreeman. @JPalakapillyKWH please proceed with the PR, and THANKS!. I suggest we expand the docstring for pvwatts_ac to better describe each input.

And, please add a note to pvwatts_dc to explain that pdc0 has a different meaning in pvwatts_dc and pvwatts_ac, but we are using the same notation to be consistent with the PVWatts technical document.

roger

Should we add dc_nameplate to module_parameters? I'm -1 on only moving pdc0 to inverter_parameters.

Also, this is going to break users' code and we've not given any warning for deprecation. I'm not sure we want to do this in 0.7

No to moving pdc0 out of module_parameters, it needs to stay. We can rename it if we're willing to diverge from the PVWatts document. OK with me as long as we add a note to the pvwatts_dc docstring.

pdc0 is already in inverter_parameters when it is created by retrieve_sam. We don't have a constant for inverter parameter names, it would be worth creating one but not in this PR.

Because this fixes a bug, do you think we need to deprecate the bug before we push the fix? Seems a little overly cautious.

Ok, I misunderstood. I don't think we need a deprecation so long as PVSystem.pvwatts_dc continues to use module_parameters['pdc0']. The ModelChain.infer_ac_model could inspect both module_parameters and inverter_parameters and emit a deprecation warning if pdc0 is only in module_parameters. Or we could look the other way on that potentially breaking change since the next release is 0.7.

Is this closed?

Closed via #735

Was this page helpful?
0 / 5 - 0 ratings

Related issues

wholmgren picture wholmgren  路  6Comments

wholmgren picture wholmgren  路  4Comments

KonstantinTr picture KonstantinTr  路  7Comments

wholmgren picture wholmgren  路  4Comments

wholmgren picture wholmgren  路  5Comments