SolarArbiter / solarforecastarbiter-core

Core data gathering, validation, processing, and reporting package for the Solar Forecast Arbiter
https://solarforecastarbiter-core.readthedocs.io
MIT License
33 stars 21 forks source link

Inconsistent terminology for "benchmark" forecasts #734

Open williamhobbs opened 3 years ago

williamhobbs commented 3 years ago

The solarforecastarbiter.org documentation uses the term "benchmark" as an official-seeming term and "reference" in the description (https://solarforecastarbiter.org/benchmarks/), while GitHub code/documentation seems to use the opposite (https://solarforecastarbiter-core.readthedocs.io/en/latest/reference-forecasts.html).

Making it consistent and/or clarifying if there is a real difference might be helpful. For example, is a benchmark forecast for a reference observation is a reference forecast, and distinct from a benchmark forecast for a non-reference observation (e.g., from a new site during a trial)?

wholmgren commented 3 years ago

This has bothered me for a couple of years. The Solar Forecasting II FOA used both terms, so we used both terms in the proposal, and then used the terms interchangeably for the first year or so. After that, I made a half-hearted attempt at the following definitions:

We also refer to the "reference database" or "reference sites". By my logic above, I'd say we prepopulate those "reference sites" with "benchmark forecasts".

Another part of the problem is that our definition of a "reference forecast" is specific to quantitative forecast skill calculations and doesn't seem to encompass qualitative comparisons.

Anyways, my feeling right now is that we should stop trying to turn "benchmark" into jargon and instead use a clear term like "arbiter forecast" or something like that. Then "reference forecast" can retain its general definition.

Would that be more clear and fit with how you think of these terms?

williamhobbs commented 3 years ago

Yeah, that makes since to me.

Would you have to change the name of the reference_forecasts subpackage?

wholmgren commented 3 years ago

Perhaps better to leave the subpackage as is for now? I considered a few other names, but in the context of the package they all felt like they were not an improvement. solarforecastarbiter.arbiter_forecasts seems redundant. solarforecastarbiter.forecasts seems vague. solarforecastarbiter.builtin_forecasts doesn't sound great. I reconsidered each after typing all them out and felt like maybe they had redeeming qualities, but ultimately I'd rather not change the API unless there's a very compelling reason to do so.

williamhobbs commented 2 years ago

I agree that it is probably better to leave as-is for now. solarforecastarbiter.forecasts or .forecast would be consistent with pvlib.forecast, but likely too vague in this context.