deep-spin / entmax

The entmax mapping and its loss, a family of sparse softmax alternatives.
MIT License
407 stars 43 forks source link

Errors when using the loss function #3

Closed berlino closed 5 years ago

berlino commented 5 years ago

The error shows that when computing the loss, the size of 'target' is not the same as 'p_star'. https://github.com/deep-spin/entmax/blob/master/entmax/losses.py#L156 Should it be switch to indexadd? Any hint?

Pytorch version: '0.4.1.post2'

Thanks

bpopeters commented 5 years ago

Thank you for opening the issue. I'm having trouble replicating this but it sounds like a mismatch in between the dimensions of the input and the target. Could you provide an example that causes this bug?

P.S. This probably won't have any effect on the bug, but that version of pytorch is rather out of date.

berlino commented 5 years ago

Thanks. After I switch to the most recent version, the error disappears. Seems like the function scatteradd has been updated.

vene commented 5 years ago

@berlino you mean the most recent pytorch version?

We should try to bisect pytorch versions and figure out our minimum requirement, to list in readme. Reopening this until we figure it out.

vene commented 5 years ago

(I suspect pytorch 1.0 is the minimum)

vene commented 5 years ago

Indeed it works with 1.0.1 which is supposed to be a minor bugfix release over 1.0, and 0.4.1 is the last release before the 1.0 line, so I will go ahead and specify pytorch>=1.0 in the requirements.

vene commented 5 years ago

Sorry to ping you again @berlino,

We added a bunch of new features to the code and we're planning to make a package release on pypi soon, would you mind checking that your application works well with the current version of the code?

Some defaults were changed (dim=-1 now everywhere) and the import paths are now shorter (from entmax import sparsemax for instance.)

berlino commented 5 years ago

No worries.

The updated version works well for my application. (I actually made similar changes when I used the code)

vene commented 5 years ago

Thanks a lot! We will make a pypi release soon.

berlino commented 5 years ago

@vene Following this thread, I think pytorch should be specified as a dependency in the setup.py, assuming the environment does not have pytorch installed.

Currently, I ran into the problem "ModuleNotFoundError: No module named 'torch'" when I specify entmax as a dependency in my our own setup.py.