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

Make irradiance input names consistent across cmods #497

Open
dguittet opened this issue Dec 15, 2020 · 5 comments
Open

Make irradiance input names consistent across cmods #497

dguittet opened this issue Dec 15, 2020 · 5 comments
Labels
enhancement pv photovoltaic, pvsam, pvwatts

Comments

@dguittet
Copy link
Collaborator

The global, beam and diffuse irradiance variables have different names across wfreader, irradproc, tcsgeneric_solar, pvwatts/pvsamv1, and pvwatts 1ts models.

In commit 9dfd506 I changed the names of the first three to be consistent, with matching commits in NREL/SAM@bd32aaa and https://github.com/NREL/SAM-private/commit/7d0e4b78ed4c4e11514f9a3bdb3dbfbead9439e8.

I am hesitant about changing the names for the PV models due to likelihood of breaking somebody's software that integrates, and figure we can either leave those alone using "diff", "beam" and "glob", or change the first three to those used by the pv models "gh", "dn" "dh".

@dguittet dguittet added the pv photovoltaic, pvsam, pvwatts label Dec 15, 2020
@cpaulgilman
Copy link
Collaborator

If we are changing variable names, I would vote for more standard ones like dni, dhi, and ghi. Those are consistent with the nsrdb, pvlib.

@dguittet
Copy link
Collaborator Author

Thanks, @cpaulgilman I like that as well, but none of the cmods listed use those standard ones. I think it's low-impact to change wfreader, irradproc, and tcsgeneric_solar, but pvwatts and pvsamv1 uses dn and df in the solar_resource_data. This would result in some folks needing to update their software eventually.

What we could do for the solar_resource_data users is to allow both dn and dni for a year or so, and throw up DEPRECATION warnings.

@janinefreeman
Copy link
Collaborator

Relevant to this, there's an initiative to help the PV modeling community standardize on certain names: https://duramat.github.io/pv-terms/ . It's currently still in draft format, but ideally we would use the finalized names from something like this across all cmods, which I believe pvlib would also plan to upgrade to as well. @cwhanse do you have any other thoughts on this? Or insight into the status of the pv terms initative?

@cwhanse
Copy link
Contributor

cwhanse commented Jan 4, 2021

Relevant to this, there's an initiative to help the PV modeling community standardize on certain names: https://duramat.github.io/pv-terms/ . It's currently still in draft format, but ideally we would use the finalized names from something like this across all cmods, which I believe pvlib would also plan to upgrade to as well. @cwhanse do you have any other thoughts on this? Or insight into the status of the pv terms initative?

pvterms is an attempt to normalize names used in software and (hopefully) also to tag data, e.g. column names in csv files. The naming patterns are python friendly, it would be good to hear if the names are a problem for other languages.

pvlib will likely adopt a majority of these terms, but it will be a long process, since changing a function parameter's name needs to go through a deprecation cycle and isn't something pvlib developers take lightly.

@dguittet
Copy link
Collaborator Author

dguittet commented Jan 7, 2021

Thanks for the thoughts. I would like to just change wfreader, irradproc, tcsgeneric_solar to use dni, dhi, and ghi within SAM for now. The variable name "global" was causing issues for the irradproc class.

For the other, much more commonly used models, we can go through the same long deprecation cycle as pvlib once we're all ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pv photovoltaic, pvsam, pvwatts
Projects
None yet
Development

No branches or pull requests

4 participants