search

WenjieDu / PyPOTS

A Python toolkit/library for reality-centric machine/deep learning and data mining on partially-observed time series, including SOTA neural network models for scientific analysis tasks of imputation/classification/clustering/forecasting/anomaly detection/cleaning on incomplete industrial (irregularly-sampled) multivariate TS with NaN missing values
https://pypots.com
BSD 3-Clause "New" or "Revised" License
1.12k stars 110 forks source link

Beta parameter is ignored in GPVAE when using the IWAE setting #540

Open AlexandreAbraham opened 1 month ago

AlexandreAbraham commented 1 month ago

Issue description

In the backbone of GPVAE, a different path is taken depending on whether IWAE is used or not.

In the case K=1, the elbo is updated like this:

elbo = -nll - self.beta * kl

In the case K>1, the elbo is updated like that:

elbo = -nll - kl

I understand that those usages are different, but it feels wrong to me to silently ignore a parameter. Also, I do not see why beta could not be applied in the IWAE case. I therefore suggest one of:

Let me know so I can propose a PR.

Thanks for the great software!

github-actions[bot] commented 1 month ago

Hi there 👋,

Thank you so much for your attention to PyPOTS! You can follow me on GitHub to receive the latest news of PyPOTS. If you find PyPOTS helpful to your work, please star⭐️ this repository. Your star is your recognition, which can help more people notice PyPOTS and grow PyPOTS community. It matters and is definitely a kind of contribution to the community.

I have received your message and will respond ASAP. Thank you for your patience! 😃

Best, Wenjie

github-actions[bot] commented 2 weeks ago

This issue had no activity for 14 days. It will be closed in 1 week unless there is some new activity. Is this issue already resolved?

AlexandreAbraham commented 2 weeks ago

Still not resolved AFAIK

AugustJW commented 2 weeks ago

Hi Alexander,

Apologies for the delayed response, and thank you very much for pointing this out!

At this stage, we would like to stay consistent with the official GP-VAE implementation. Additionally, there doesn't appear to be any established foundation or implementation for combining beta with IWAE. Given this, we believe the most appropriate solution would be to raise a warning when K > 1 and beta is specified, as you suggested.

If you're interested in proceeding with a PR to implement this warning, we'd be happy to review it. Thanks again for your valuable contribution to improving the software!

AlexandreAbraham commented 2 weeks ago

No problem, it's a very minor issue, I understand that it's not a priority ;). I'll push a PR soon.

github-actions[bot] commented 2 days ago

This issue had no activity for 14 days. It will be closed in 1 week unless there is some new activity. Is this issue already resolved?