Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nn_loss is not exported #1281

Closed
tdhock opened this issue Feb 18, 2025 · 4 comments
Closed

nn_loss is not exported #1281

tdhock opened this issue Feb 18, 2025 · 4 comments

Comments

@tdhock
Copy link

tdhock commented Feb 18, 2025

Hi! First of all, thanks very much for maintaining torch, which I find very useful in my machine learning research.

I'm trying to define a nn_module based on my own custom loss function.

I looked in nn-loss.R and adapted the code therein to get

my_mse_loss <- torch::nn_module(
  "my_mse_loss",
  inherit = torch::nn_loss,
  initialize = function(reduction = "mean") {
    super$initialize(reduction = reduction)
  },
  forward = function(input, target) {
    torch::nnf_mse_loss(input, target, reduction = self$reduction)
  }
)

but when running the code above I get the error below

> my_mse_loss <- torch::nn_module(
+ "my_mse_loss",
+ inherit = torch::nn_loss,
+ initialize = function(reduction = "mean") {
+ super$initialize(reduction = reduction)
+ },
+ forward = function(input, target) {
+ torch::nnf_mse_loss(input, target, reduction = self$reduction)
+ }
+ )
Erreur : 'nn_loss' nest un object exporté depuis 'namespace:torch'

I expected that the code above should work without error, so the user could be able to define their own loss modules.
The error above indicates that nn_loss is not exported.
Can nn_loss be exported please?
If not, what is the recommended method to define our own loss modules?

A work-around is below, inherit=nn_mse_loss (which is exported, but are we supposed to inherit from nn_mse_loss?)

my_mse_loss <- torch::nn_module(
  "my_mse_loss",
  inherit = torch::nn_mse_loss,
  initialize = function(reduction = "mean") {
    super$initialize(reduction = reduction)
  },
  forward = function(input, target) {
    torch::nnf_mse_loss(input, target, reduction = self$reduction)
  }
)
@dfalbel
Copy link
Member

dfalbel commented Feb 24, 2025

@tdhock I don't think you need to inheirt from nn_loss, just a bare nn_module seems fine to me:

my_mse_loss <- torch::nn_module(
  "my_mse_loss",
  initialize = function(reduction = "mean") {
    super$initialize(reduction = reduction)
  },
  forward = function(input, target) {
    torch::nnf_mse_loss(input, target, reduction = self$reduction)
  }
)

There's nothing really special about the nn_loss module.

@tdhock
Copy link
Author

tdhock commented Feb 25, 2025

Thanks!
if that is the case, why does the current definition of nn_mse_loss have inherit=nn_loss?
According to your advice, the inherit could be removed and it still should work fine, right?

@dfalbel
Copy link
Member

dfalbel commented Feb 25, 2025

Yes, the only difference is that it doesn't need self$reduction <- reduction and instead uses self$initialize(reduction).
Honestly, there's no clear win on the approach we used here versus just doing:

my_mse_loss <- torch::nn_module(
  "my_mse_loss",
  initialize = function(reduction = "mean") {
   self$reduction <- reduction
  },
  forward = function(input, target) {
    torch::nnf_mse_loss(input, target, reduction = self$reduction)
  }
)

torch/R/nn-loss.R

Lines 374 to 384 in abd2a09

nn_mse_loss <- nn_module(
"nn_mse_loss",
inherit = nn_loss,
initialize = function(reduction = "mean") {
super$initialize(reduction = reduction)
},
forward = function(input, target) {
nnf_mse_loss(input, target, reduction = self$reduction)
}
)

@tdhock
Copy link
Author

tdhock commented Feb 26, 2025

ok great thanks

@tdhock tdhock closed this as completed Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants