search

kiudee / cs-ranking

Context-sensitive ranking and choice in Python with PyTorch
https://cs-ranking.readthedocs.io
Apache License 2.0
66 stars 15 forks source link

MultinomialLogitModel silently chooses l2 for unknown regularization #128

Closed helegraf closed 4 years ago

helegraf commented 4 years ago

If the given regularization is not "l1" or "l2", "l2" is chosen, which seems undesirable as the parameter already has "l2" as the default value so None values do not need to be accounted for.

https://github.com/kiudee/cs-ranking/blob/49e39df3257679acb157b4ca0f47a8bfaa73866a/csrank/discretechoice/multinomial_logit_model.py#L68-L71

kiudee commented 4 years ago

Good find! This is a similar problem to #122, which we recently fixed.

timokau commented 4 years ago

Thanks for reporting! I only checked for not in in the fix for #122, turns out I should have checked for the reverse as well. This pattern is a bit harder to screen for, but I hope I caught all in #133.

The many duplicate lines containing the issue again shows that we should probably work on some deduplication, probably as part of #125.