araffin / srl-zoo

State Representation Learning (SRL) zoo with PyTorch - Part of S-RL Toolbox
https://srl-zoo.readthedocs.io/en/latest/
MIT License
162 stars 18 forks source link

fixed a bug when multi split states are used #25

Closed GaspardQin closed 5 years ago

GaspardQin commented 5 years ago

Problem

When multiple split states are used, like autoencoder:188 reward:10 inverse:2, the origin version will fail. Because srl_model_knn is re-initialized.

How to fix it

Change srl_model_knn to a dictionary. (Not sure about line 108)

araffin commented 5 years ago

Does this version work still when using several losses on the same dimension?

GaspardQin commented 5 years ago

Actually, yes. From line 78 to 82,

    if split_dimensions is not None and isinstance(split_dimensions, OrderedDict):
        for loss_name, loss_dim in split_dimensions.items():
            print(loss_name, loss_dim)
            if loss_dim > 0 or len(split_dimensions) == 1:
                loss_dims[loss_name] = loss_dim

The negative dimension's loss are not added to loss_dims, so it won't cause any problem in this change.

However, the negative dimension's loss name will not be shown in the fig_name (line 108), I'm not sure whether this is a problem.

araffin commented 5 years ago

Ok, then, merging.