openclimatefix / pv-site-production

Production service for PV site level forecasts
MIT License
1 stars 3 forks source link

negative forecasts #60

Open peterdudfield opened 1 year ago

peterdudfield commented 1 year ago

Detailed Description

Would be good to make sure we cant return negative forecasts

Context

Possible Implementation

simlmx commented 1 year ago

I feel the model should be responsible to not return negative values. We could also filter or warn in other places but I'd be afraid we don't notice the eventual problems with models. To me this falls under bad predictions which we should be able to detect anyway. In other words for now I would simply fix it in the model and that's it.

peterdudfield commented 1 year ago

I perhaps see it more in a risk way. We dont want the user to see negative forecasts, how do we do this.

simlmx commented 1 year ago
  • What about if there is bug, and the model misses something, should we have another mitigation against it? This could be in the datamodel

In any case we need some sort of validation step before we put a new model in production. This is when those could be found.

  • We could also something in the read function SUM(GREATEST(ordered_item.amount, 0)) as purchases or something like that

This could slow things down but especially this could hide a bug.

The question is which is worse: having the user see a negative value and tell us that there is an issue, or returning a negative value that gets silently changed into 0 and we don't know that there is an issue.


We can always put the check back in the pydantic model, and, after we have fixed the model. If a model were to return 0 ever again, it will crash there and be found in the validation step I describe anyway.

peterdudfield commented 1 year ago

Ok, is it it fixed in the model?

simlmx commented 1 year ago

Not yet, it still returns -0.001 kind of values once in a while.

peterdudfield commented 1 year ago

ok, probably worth clipping somewhere

peterdudfield commented 1 year ago

@simlmx did you add this to the forecast?

simlmx commented 1 year ago

Not yet. Seemed low priority as the negative values are pretty much invisible in the UI (since they are so close to 0).