datawhalechina / torch-rechub

A Lighting Pytorch Framework for Recommendation Models, Easy-to-use and Easy-to-extend.
MIT License
374 stars 69 forks source link

Default argument value is mutable #24

Closed InEase closed 2 years ago

InEase commented 2 years ago

I notice that mutable Default argument value is used in rechub, which may cause unexpected results that are difficult to discover. It will share the mutable object in all instances.

Ref: python document

Default parameter values are evaluated from left to right when the function definition is executed. This means that the expression is evaluated once, when the function is defined, and that the same “pre-computed” value is used for each call. This is especially important to understand when a default parameter value is a mutable object, such as a list or a dictionary: if the function modifies the object (e.g. by appending an item to a list), the default parameter value is in effect modified. This is generally not what was intended. A way around this is to use None as the default, and explicitly test for it in the body of the function, e.g.:


def whats_on_the_telly(penguin=None):
    if penguin is None:
        penguin = []
    penguin.append("property of the zoo")
    return penguin
``
wangych6 commented 2 years ago

@InEase This is a very good question. We use mutable objects as default parameters to facilitate understanding and use. In order to avoid unforeseen problems, we ensure that all functions that use mutable objects as parameters do not modify mutable objects. That is, it is used as a ‘const ’ object.

morningsky commented 2 years ago

@InEase Yes, we have noticed this problem. For the motivation of easy-to-read, we keeps this style before. Because the most of mutable parameters as DNN Uints, it usually not be update after defined the network. Thanks for your suggestion.