caow13 / BRITS

Code of NIPS18 Paper: BRITS: Bidirectional Recurrent Imputation for Time Series
MIT License
196 stars 69 forks source link

forward deltas == backward deltas? #12

Open MaciejSkrabski opened 2 years ago

MaciejSkrabski commented 2 years ago

The deltas are identical in both directions

I do not know if this is intended. I do not see it justified in the paper. This means, that backward deltas do not apply to backward mask. I have noticed you have inverted masks twice for backward input processing:

  1. first you invert masks when sending an argument to parse_rec function;
  2. then you reverse masks again in parsedelta in ```if dir == 'backward'```

So whatever bidirectional thing you calculate, the deltas do not align, so the temp decay is wrong, so the gammas... and so on. Allow me to suggest creating tests using single dimensional data.

The-Black-Coat commented 2 years ago

I noticed the same issue.

Would a simple fix be to remove the following lines in the parse_delta() function (located in input_process.py) ?? :

""" if dir_ == 'backward': masks = masks[::-1] """

And keep the following line without any change (except removing the argument "dir_='backward' " obviously) : rec['backward'] = parse_rec(values[::-1], masks[::-1], evals[::-1], eval_masks[::-1])

Yovoss commented 1 year ago

I encountered the same question, it seems to be wrong.