-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update CEC module and inverter tables with latest SAM files #1433
base: main
Are you sure you want to change the base?
Conversation
Not sure why the checks aren't showing up for the latest commit; github's webhooks have been having issues this week, but githubstatus.com doesn't show anything amiss at the moment. Closing and reopening... |
I guess this is ready for review. I'm still very confused about the missing rows. For example there's not a single Canadian Solar module in either of these two tables on the CEC website, so they're not in these SAM files either. Am I missing something? |
Could we have a file that is the union of the current and older files? Perhaps with a date or version label added? |
A merged file seems like the best outcome, but it might be difficult to achieve. See #761 (comment) and https://sam.nrel.gov/forum/forum-general/3779-missing-pv-modules-and-inverters.html When they open in a few hours I will call the CEC to ask about these removals; maybe there is some mistake. |
The CEC has been removing equipment from their list when certifications expire. https://www.energy.ca.gov/files/equipment-noticed-removal Some time ago, I had a conversation wtih SAM developers about retaining obsolete listings. The compromise was to retain old versions of the SAM files but I've lost track of that location. |
I doubt it's technically difficult. Alternatively pvlib can just add rather than replace new versions of the files. |
It would seem not, but in fact is an onerous task to merge these files. The NREL process relies on the CEC list's manufacturer and equipment identifiers to assign a key, and CEC changes the format of those fields every few years. To make matters worse, submitters provide the values so when a company or brand changes hands, the module identifier may or may not change. The result is the same module with different keys; to avoid duplicate or competing entries, one would have to merge these lines manually.
Yes, at the risk of getting user questions about different parameter sets and record structures. I am convinced the long-term solution is to build out pvlib/pvmodules to have public code that generates the parameters, and to curate the inputs by frequently pulling the CEC list, identifying new items, and adding the content to a pvmodules database, thus accumulating an archival list in a common format. |
To make my view clear, @adriesse suggested "Alternatively pvlib can just add rather than replace new versions of the files." For this PR, I agree with this suggestion. |
|
this might break existing code, but my preference would be to reference the file by name. Maybe not even ship it with pvlib, but tell folks you can retrieve from NREL/SAM GitHub repo: For example:
Are there missing steps, EG: is the SAM file pre-processed somehow for pvlib? Another option, would be to leverage the
there's also a |
The issue here is that SAM decided to replace the file at each SAM release, so the latest version has the missing/dropped module problem. The archive of parameters for modules dropped by the CEC is the set of files for previous SAM versions.
that's a good idea. |
I've added a Edit: I couldn't get the CEC office on the phone so I emailed them instead. No reply yet. |
Personally I would recommend posting those files somewhere online (zenodo?), removing them from the distribution, and forcing users to manually specify a file path. (I think this is similar to Mark's first suggestion). I'm not too thrilled with complicating the pvlib api for file access. But no worries if you want to go this way. |
I believe I can crawl their GitHub history to retrieve the older files
That’s half of what pvfree already does. If only I had more time to work on it. And I feel your pain, merging and parsing constantly shifting model & mfg names is why I made the database to begin with |
[ ] Updates entries indocs/sphinx/source/reference
for API changes.docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).[ ] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.It's a little concerning how many rows got dropped in each table (compare the line counts in the diff). A very brief and non-comprehensive comparison suggests that the upstream CEC files did indeed drop a lot of rows, so I guess it is what it is.