wwrechard / pydlm

A python library for Bayesian time series modeling
BSD 3-Clause "New" or "Revised" License
475 stars 98 forks source link

predictN modifies dlm object #23

Open albertotb opened 6 years ago

albertotb commented 6 years ago

Hi all,

The steps to reproduce my problem are:

  1. Create an empty dlm with trend component
  2. Add elements to the model in a loop using append and fit the model
  3. Version 1: use predictN at the end of the loop, getting one prediction
  4. Version 2: use predictN in each iteration, getting one prediction per iteration

I would expect the prediction in step 3 and the last prediction in step 4 to be the same. However, it seems that somehow predictN is changing the dlm object, which does not make sense to me. What am I missing? Installed versions:

I attach the code to reproduce the problem below:

# coding: utf-8

import pandas as pd
from pydlm import dlm, trend

ts = [
 0.5429682543922109,
 0.5296058346035057,
 0.5403294585554494,
 0.542441925561093,
 0.5435209708555084,
 0.5430676782288945,
 0.5429877208796179,
 0.5429721282202071,
 0.5429690254184671,
 0.5449758960859548,
 0.5457612294317765,
 0.5434065016617284,
 0.5430519745276086,
 0.5436459000038072,
 0.5437794184525637]

## Version 1
model = dlm([]) + trend(degree=2, discount=0.95, name='trend1')

d = {}
for idx, el in enumerate(ts):
    model.append([el], component='main')
    model.fitForwardFilter()

mean, var = model.predictN(N=1, date=model.n-1)
d[idx] = mean

df1 = pd.DataFrame.from_dict(d, orient="index")

## Version 2
model = dlm([]) + trend(degree=2, discount=0.95, name='trend1')

d = {}
for idx, el in enumerate(ts):
    model.append([el], component='main')
    model.fitForwardFilter()
    mean, var = model.predictN(N=1, date=model.n-1)
    d[idx] = mean

df2 = pd.DataFrame.from_dict(d, orient="index")

print(df1)
print(df2)

Thanks in advance

wwrechard commented 6 years ago

Hi albertotb,

Thanks for identifying the issue. This is a bug and should have been fixed in my most recent push. The prediction function was previously implemented based on the assumption that there is no follow-up operations. So it will change the model status for re-using some of the model structure. I now diverted the prediction calculation into a shadow model, so that the status of the main model won't change.

Could you give it a try with the most recent github version? Thanks!

albertotb commented 6 years ago

It seems that copying the model beforehand did the trick, in this version it works perfectly. Thanks!