Dnn2bnn#11
Conversation
piEsposito
left a comment
There was a problem hiding this comment.
I think we should assign a value to kl even if it is null, so it won't break when we are integrating to already working PyTorch models and training scripts.
Also, maybe we should add unit-tests, rebase this branch on it and check if that won't break things.
What do you think @ranganathkrishnan ?
|
|
||
| kl = self.kl_div(self.mu_kernel, sigma_weight, self.prior_weight_mu, | ||
| self.prior_weight_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
There was a problem hiding this comment.
Hey @piEsposito , Do you have any specific scenarios in mind that this would break? With these changes, kl can be computed in kl_loss() function (https://github.com/IntelLabs/bayesian-torch/pull/11/files#diff-a588ca678816135810210bacf4645009ff6ecd3f16626713fd576db4aa603b07R126). For example, see https://github.com/IntelLabs/bayesian-torch/pull/11/files#diff-ade00648d59ef89627b50d68cb88f47ffc6d4832dd4c366902285fce80fec709R157 .
| bias = (sigma_bias * eps_bias) | ||
| kl = kl + self.kl_div(self.mu_bias, sigma_bias, self.prior_bias_mu, | ||
| self.prior_bias_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Idem here: Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
|
|
||
| kl = self.kl_div(self.mu_kernel, sigma_weight, self.prior_weight_mu, | ||
| self.prior_weight_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
| bias = (sigma_bias * eps_bias) | ||
| kl = kl + self.kl_div(self.mu_bias, sigma_bias, self.prior_bias_mu, | ||
| self.prior_bias_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
|
|
||
| kl = self.kl_div(self.mu_kernel, sigma_weight, self.prior_weight_mu, | ||
| self.prior_weight_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
| weight = self.mu_kernel + (sigma_weight * eps_kernel) | ||
| kl_weight = self.kl_div(self.mu_kernel, sigma_weight, | ||
| self.prior_weight_mu, self.prior_weight_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
| bias = self.mu_bias + (sigma_bias * eps_bias) | ||
| kl_bias = self.kl_div(self.mu_bias, sigma_bias, self.prior_bias_mu, | ||
| self.prior_bias_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
| weight = self.mu_kernel + (sigma_weight * eps_kernel) | ||
| kl_weight = self.kl_div(self.mu_kernel, sigma_weight, | ||
| self.prior_weight_mu, self.prior_weight_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
| bias = self.mu_bias + (sigma_bias * eps_bias) | ||
| kl_bias = self.kl_div(self.mu_bias, sigma_bias, self.prior_bias_mu, | ||
| self.prior_bias_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
| bias = self.mu_bias + (sigma_bias * self.eps_bias.data.normal_()) | ||
| kl_bias = self.kl_div(self.mu_bias, sigma_bias, self.prior_bias_mu, | ||
| self.prior_bias_sigma) | ||
| if return_kl: |
There was a problem hiding this comment.
Hey, I think only computing the kl value if return_kl is set to True might break things if we are plugging bayesian layers on a deterministic model.
Maybe the best way could be if self.dnn_to_bnn_flag is set to True, we set it to null, but not computing it at all might just break everything - and we don't have unit tests yet to ensure things won't break.
Hi @piEsposito , I agree to include the unit-tests, but that can be done on top of this PR. @msubedar and I have validated all the existing scripts for training and evaluating the models in examples folder, so that the new feature does not break older model versions (i.e. Bayesian models defined without dnn_to_bnn()). |
PR supports DNN to BNN auto conversion. One can create a torch.nn DNN model and call dnn_to_bnn() to convert it to BNN model. An example script examples/main_bayesian_cifar_dnn2bnn.py is provided.