L1 loss added to the models #85

soumickmj · 2023-04-05T10:33:02Z

MSE loss sometimes make the models produce smooth images. L1 loss is an easy drop-in fix for the same. I have added to the models where we already had MSE and BCE loss functions, and skipped (for now) the ones without the recon loss flag. In the futurue, I will also add a feature to pass custom loss functions.

the values are clamped before sqrt to avoid NaN during training, as done in the original implementation https://github.com/nicola-decao/s-vae-pytorch/blob/master/hyperspherical_vae/distributions/von_mises_fisher.py#L68

clementchadebec

Hi @soumickmj,
Thank you very much for this contribution and sorry for the late reply. Please see some comments that, in my opinion, should be addressed before merging.

First, we need to allow to pass l1 as argument of the VAEConfig (see comment on vae_config.py)
Second, since the l1 loss does not really refer to a distribution (e.g. BCE = Bernoulli, MSE = multivariate standard), we should not allow the computation of the NLL when such a loss is chosen. Instead, I propose to add an exception is get_nll method is called with l1 as loss as follows

raise NotImplementedError("Computation of the likelihood is not implemented when `L1 loss` is chosen")

I any case, do not hesitate if you have any questions or do not agree with the proposed modifications.

Best,

Clément

clementchadebec · 2023-04-11T09:44:47Z

src/pythae/models/svae/svae_model.py

@@ -286,6 +294,18 @@ def get_nll(self, data, n_samples=1, batch_size=100):
                        reduction="none",
                    ).sum(dim=-1)

+                elif self.model_config.reconstruction_loss == "l1":


The computation of the likelihood cannot be handled with the l1 loss since it does not correspond to a tractable distribution per say.

clementchadebec · 2023-04-11T09:44:58Z

src/pythae/models/rhvae/rhvae_model.py

@@ -572,6 +586,18 @@ def get_nll(self, data, n_samples=1, batch_size=100):
                        reduction="none",
                    ).sum(dim=-1)

+                elif self.model_config.reconstruction_loss == "l1":


The computation of the likelihood cannot be handled with the l1 loss since it does not correspond to a tractable distribution per say.

clementchadebec · 2023-04-11T09:45:33Z

src/pythae/models/vae/vae_config.py

@@ -11,7 +11,7 @@ class VAEConfig(BaseAEConfig):
    Parameters:
        input_dim (tuple): The input_data dimension.
        latent_dim (int): The latent space dimension. Default: None.
-        reconstruction_loss (str): The reconstruction loss to use ['bce', 'mse']. Default: 'mse'
+        reconstruction_loss (str): The reconstruction loss to use ['bce', 'l1', 'mse']. Default: 'mse'
    """

    reconstruction_loss: Literal["bce", "mse"] = "mse"


This should be replaced by

reconstruction_loss: Literal["bce", "mse", "l1"] = "mse"

clementchadebec · 2023-04-11T09:47:59Z

src/pythae/models/rhvae/rhvae_model.py

@@ -424,6 +424,20 @@ def _log_p_x_given_z(self, recon_x, x):
                reduction="none",
            ).sum(dim=-1)

+        elif self.model_config.reconstruction_loss == "l1":


For this model, this should correspond to a known distribution. The mse actually models a multivariate normal. In the case of L1 loss, there is not really an underlying distribution so we should not allow the L1 for this model.

clementchadebec · 2023-04-11T09:48:33Z

src/pythae/models/rhvae/rhvae_config.py

@@ -9,6 +9,7 @@ class RHVAEConfig(VAEConfig):

    Parameters:
        latent_dim (int): The latent dimension used for the latent space. Default: 10
+        reconstruction_loss (str): The reconstruction loss to use ['bce', 'l1', 'mse']. Default: 'mse'


l1 cannot be allowed in this model (see next comment)

clementchadebec · 2023-04-11T09:49:36Z

src/pythae/models/vae/vae_model.py

@@ -186,6 +194,18 @@ def get_nll(self, data, n_samples=1, batch_size=100):
                        reduction="none",
                    ).sum(dim=-1)

+                elif self.model_config.reconstruction_loss == "l1":


Let's not compute the likelihood when l1 loss is chosen since there is no associated distribution. We may add a warning with the following message

raise NotImplementedError("Computation of the likelihood is not implemented when `L1 loss` is chosen")

clementchadebec · 2023-04-11T09:52:05Z

src/pythae/models/vae_iaf/vae_iaf_model.py

@@ -212,6 +220,18 @@ def get_nll(self, data, n_samples=1, batch_size=100):
                        reduction="none",
                    ).sum(dim=-1)

+                elif self.model_config.reconstruction_loss == "l1":


Same as before for the computation of the NLL.

clementchadebec · 2023-04-11T09:52:30Z

src/pythae/models/vae_lin_nf/vae_lin_nf_model.py

@@ -226,6 +234,18 @@ def get_nll(self, data, n_samples=1, batch_size=100):
                        reduction="none",
                    ).sum(dim=-1)

+                elif self.model_config.reconstruction_loss == "l1":


Same as before for the computation of the NLL.

clementchadebec · 2023-04-11T09:52:53Z

src/pythae/models/vamp/vamp_model.py

@@ -243,6 +251,18 @@ def get_nll(self, data, n_samples=1, batch_size=100):
                        reduction="none",
                    ).sum(dim=-1)

+                elif self.model_config.reconstruction_loss == "l1":


Same as before for the computation of the NLL.

clementchadebec · 2023-04-11T09:54:06Z

tests/test_RHVAE.py

@@ -38,6 +38,9 @@ def model_configs_no_input_dim(request):
        RHVAEConfig(
            input_dim=(1, 28, 28), latent_dim=1, n_lf=1, reconstruction_loss="bce"
        ),
+        RHVAEConfig(


Not needed since this model will not handled L1 loss

Custom loss plan cancelled

soumickmj and others added 10 commits February 7, 2023 17:58

predict method in the base, hmogenising recon_loss

905a9e2

minor modifications for PR 75

1a8652c

Merge branch 'clementchadebec:main' into main

9938bae

Merge branch 'clementchadebec:main' into main

a0269b7

Merge branch 'clementchadebec:main' into main

d4d835e

Merge branch 'clementchadebec:main' into main

6080a8f

L1 loss added to the models

472e3a3

comments for configs updated

44bbd5c

custom loss function added to factor VAE

6ad1c0a

clementchadebec requested changes Apr 11, 2023

View reviewed changes

soumickmj and others added 12 commits May 23, 2023 12:30

boh

518f64a

boh

e01360d

cvcnn added

ee4a8c2

Merge branch 'clementchadebec:main' into main

0b9283c

vae config l1 recon loss added

151f7f3

reverting factor VAE back to original

101dafb

Merge pull request #2 from soumickmj/custom_loss

20872bc

Custom loss plan cancelled

Merge branch 'clementchadebec:main' into main

6394636

ae_model updated with recon loss

cc0078e

ae_config updated with recon loss param

d1739e3

minor bug fix in ae_config.py

e7c96a8

predict added in rhvae

c85c8f6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

L1 loss added to the models #85

L1 loss added to the models #85

soumickmj commented Apr 5, 2023

clementchadebec left a comment

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

clementchadebec Apr 11, 2023

L1 loss added to the models #85

Are you sure you want to change the base?

L1 loss added to the models #85

Conversation

soumickmj commented Apr 5, 2023

clementchadebec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment