NREL / ssc

SAM Simulation Core (SSC) contains the underlying performance and financial models for SAM
BSD 3-Clause "New" or "Revised" License
79 stars 85 forks source link

Solar Resource Data does not throw errors for bad fields #973

Open brtietz opened 1 year ago

brtietz commented 1 year ago

Describe the bug A PySAM user was attempting to use Solar Resource Data to drive a PV simulation. The simulation provided worked fine with a weather file, but the solar resource data had a bad key "Elevation" instead of "elev". No error was thrown, but DC power was not calculated.

To Reproduce TODO: upload example workbook

Expected behavior The code should throw errors for this case.

Desktop (please complete the following information):

mjprilliman commented 1 year ago

I wasn't able to duplicate this behavior when running example scripts. The solar_resource_data did not have an 'elev' key but the simulation still ran and generated dc power results and gen results. The same happened when I properly added an 'elev' key. I think there's enough uncertainty on what should be reported and at what verbosity level to maybe punt this to the release? @cpaulgilman @dguittet what should be printed when optional parameters aren't added to the resource dictionaries?

cpaulgilman commented 1 year ago

I wasn't able to duplicate this behavior when running example scripts. The solar_resource_data did not have an 'elev' key but the simulation still ran and generated dc power results and gen results. The same happened when I properly added an 'elev' key. I think there's enough uncertainty on what should be reported and at what verbosity level to maybe punt this to the release? @cpaulgilman @dguittet what should be printed when optional parameters aren't added to the resource dictionaries?

For weather files, the wfcheck module is available to check solar resource data, but there is not a similar module for checking data provided as a table. I agree with moving this to the Fall 2023 release given the level of effort to fix it. Some notes:

cpaulgilman commented 2 months ago

When you use the weather data option with no weather file, the only checks are done by the performance model (pvwattsv8, pvsamv1, etc.), which calls irrad::check() in irradproc to provide feedback about the weather data. For example, for data with an invalid DNI value, pvwattsv8 reports the cryptic “Error: exec fail(pvwattsv8): Failed to process irradiation on surface (code: -105) [year:2004 month:1 day:1 hour:12 minute:30]. time -1.000000”.

For header data, the irrad::check() module has a check for lat/lon but not elevation or time zone.

I think the solution is:

  1. Report descriptions of errors instead of error codes.
  2. Make checking of weather data consistent for weather file input and weather data input.