search

sktime / sktime-dl

DEPRECATED, now in sktime - companion package for deep learning based on TensorFlow
BSD 3-Clause "New" or "Revised" License
595 stars 79 forks source link

modified the loss function for _cnn.py #128

Closed Vasudeva-bit closed 9 months ago

Vasudeva-bit commented 2 years ago

Reference Issues/PRs

Not an issue fix

What does this implement/fix? Explain your changes.

I have modified the loss function of the _cnn.py file. The current loss function for the _cnn.py file which is used to classify between two classes is RMSE but the appropriate loss function for training a model for binary classification is binary_crossentrophy. This loss function can significantly improve the training and performance of the estimator.

Does your contribution introduce a new dependency? If yes, which one?

No

Any other comments?

No

Vasudeva-bit commented 9 months ago

Hello @fkiraly, Kindly review this pr, if this looks valid, I will go ahead and do a new pr to sktime.

fkiraly commented 9 months ago

Yes, looks like a bug. Simplest way, I would proceed as you suggest. MSE is not appropriate for non-ordinal multiclass, but it is the same as Brier loss for binary probabilistic classification.

Perhaps it would be slightly nicer with an option to choose the loss as well, from the __init__, though I would not deem that necessary for the fix.

Vasudeva-bit commented 9 months ago

Perhaps it would be slightly nicer with an option to choose the loss as well, from the __init__

As you suggested, it's better to let user choose the loss function, I shall modify accordingly. I think, by default, loss function is cross-entropy loss, which should just work fine in all scenarios.

fkiraly commented 9 months ago

Thanks. Obsolete as replaced by https://github.com/sktime/sktime/pull/5852 in sktime.