NREL / reV

Renewable Energy Potential (reV) Model
https://nrel.github.io/reV/
BSD 3-Clause "New" or "Revised" License
107 stars 24 forks source link

Add resource length warning for easier debugging #457

Closed ppinchuk closed 5 months ago

ppinchuk commented 5 months ago

@grantbuster Reid recently ran into a tough-to-debug issue where SAM wasn't setting the output variables correctly, and it came down to the fact that he was using time_index_step=2 on hourly resource data.

Since this was a tough problem to track down, I figured I would add a few warning messages if this case is detected. Any objections? Any reason why someone would want to run less than a full 8760 time series through SAM? Is that even allowed (seems like not based on this experience but I haven't fully tested that)?

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (main@e92af3a). Learn more about missing BASE report.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #457 +/- ## ======================================= Coverage ? 87.57% ======================================= Files ? 122 Lines ? 18086 Branches ? 0 ======================================= Hits ? 15839 Misses ? 2247 Partials ? 0 ``` | [Flag](https://app.codecov.io/gh/NREL/reV/pull/457/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/NREL/reV/pull/457/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL) | `87.57% <100.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ppinchuk commented 5 months ago

Ah good to know. Interested to see if reV can handle the monthly case out of the box or not. We should probably set up a test for that when it becomes relevant - might be a good time to re-evaluate this message too :)

(Happy to take charge on that whenever it comes around)