For options that only takes one argument, It would be awesome to use a similar design as https://github.com/pytorch/pytorch/pull/27435/files#diff-1e16803d96151e8f6eb7a327394b6584R88-R94 (i.e. adding an implicit constructor and using the constructor to set the default for reduction).

It would be awesome to give a default value {} to options (and add a test case for the "no options" use case), similar to the other PRs :D

It would be awesome to call the functional form to match the Python version even better:

nit: it would be awesome to align the indentation with the first branch of the if block :D

nit: we could remove the new line here

It would be awesome to add a test for this as well :D

As part of the pretty_print test, we can also test that SmoothL1Loss(/*reduction=*/Reduction::Mean) (i.e. the implicit constructor of SmoothL1LossOptions) works :D

EDIT: Actually, I am just now realizing (while working with the python API) that reductions are never printed... So I guess I will just remove this.

Hi @yf225, first of all, thanks for the reviews!
I had deliberately left the pretty_print tests out, since there were no such tests for the L1Loss, which has the same and unique parameter (reduction) as SmoothL1Loss, MultiMarginLoss and SoftMarginLoss. I guessed this was due to #25883, where you pointed out

there are still a lot of preparation work that needs to be done (e.g. fixing how we handle enums in C++ API #15149).

and these losses use the Reduction::Reduction enum... Should I use a switch statement like the one below?

void SmoothL1LossImpl::pretty_print(std::ostream& stream) const {
  stream << "torch::nn::SmoothL1Loss(reduction=";
  switch(options.reduction()) {
    case Reduction::None:
      stream << "Reduction::None";
    break;
    case Reduction::Mean:
      stream << "Reduction::Mean";
    break;
    case Reduction::Sum:
      stream << "Reduction::Sum";
    break;
  }
  stream << ")";
}

We can remove SmoothL1LossOptions() in this line to test the "no options" use case :D

As part of the pretty_print test, we can also test that SmoothL1Loss(/*reduction=*/Reduction::Mean) (i.e. the implicit constructor of SmoothL1LossOptions) works :D

We could write this as

to also test the implicit conversion for SmoothL1LossOptions :D

Hi @yf225! Taking your previous reviews into account, I wrote these 4 tests for each single-optional-parameter-loss module/functional pair, covering explicit instantiation (in modules.cpp, *DefaultOptions), explicit instantiation + changing parameter value (in modules.cpp, *NoReduction, this test case), omitting optional parameter for functional (in functional.cpp, *DefaultOptions) and implicit instantiation as a parameter in functional (in functional.cpp, *NoReduction).

Do you see any other test that should be run for these modules/functionals and their options? (I ask this here because you already corrected the others hehe)