Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequence-based indexing in theano is deprecated #74

Closed
timokau opened this issue Nov 14, 2019 · 3 comments · Fixed by #76
Closed

Sequence-based indexing in theano is deprecated #74

timokau opened this issue Nov 14, 2019 · 3 comments · Fixed by #76

Comments

@timokau
Copy link
Collaborator

timokau commented Nov 14, 2019

In csrank/discretechoice/nested_logit_model.py we make use of sequence indexing in theano multiple times. For example:

rows, cols = tt.eq(self.y_nests, i).nonzero()

Here rows and cols are both 1d tensors which are then used to index a different tensor:

utility = tt.set_subtensor(utility[rows, cols], tt.dot(self.Xt[rows, cols], weights[i]))

Theano complains that this is deprecated:

FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result.

I'm not sure how to fix this. According to the documentation, theano should support boolean mask indexing. So I thought we should be able to do

mask = tt.eq(self.y_nests, i)
utility = tt.set_subtensor(utility[mask], tt.dot(self.Xt[mask], weights[i]))

instead (as tt.eq should return a boolean mask). But unfortunately that doesn't work; it gives the same warning.

Any ideas?

@timokau timokau mentioned this issue Nov 14, 2019
7 tasks
@timokau timokau changed the title Sequence-based intexing in theano is deprecated Sequence-based indexing in theano is deprecated Nov 14, 2019
@kiudee
Copy link
Owner

kiudee commented Nov 18, 2019

I think this problem was caused by a new version of NumPy and has been discussed here:
Theano/Theano#6703

@timokau
Copy link
Collaborator Author

timokau commented Nov 18, 2019

Right, so the api is still supported by theano. I've confirmed that the issue is fixed in upstream by running the testsuite with a patched theano. Another case of "not our fault" then.

@timokau
Copy link
Collaborator Author

timokau commented Nov 18, 2019

For us, the commits 082yqypw8vddxly47p26l9sgda3z598i098dldp2d0k04vawlblx and 0y2g08arbcffgkd9w0aan1g2yn7n06ma8qwd8i2vvbj5z1n28xvq are interesting (not included in the PR you linked, but similar in intention).

timokau added a commit to timokau/cs-ranking that referenced this issue Nov 18, 2019
kiudee pushed a commit that referenced this issue Nov 19, 2019
prithagupta pushed a commit that referenced this issue Nov 26, 2019
prithagupta pushed a commit that referenced this issue Nov 26, 2019
Out of our control.

Fixes #74

removed a bug due to newer version of keras
prithagupta pushed a commit that referenced this issue Dec 12, 2019
Out of our control.

Fixes #74

removed a bug due to newer version of keras
prithagupta pushed a commit that referenced this issue Feb 14, 2020
Out of our control.

Fixes #74

removed a bug due to newer version of keras
prithagupta pushed a commit that referenced this issue Mar 11, 2020
Out of our control.

Fixes #74

removed a bug due to newer version of keras
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants