search

google-research / torchsde

Differentiable SDE solvers with GPU support and efficient sensitivity analysis.
Apache License 2.0
1.52k stars 195 forks source link

Use of `np.searchsorted` in `BrownianTree`. #29

Closed patrick-kidger closed 3 years ago

patrick-kidger commented 3 years ago

I think this line is slightly wrong: https://github.com/google-research/torchsde/blob/381f4e9854b275faa5629915c1f805bf78090dd8/torchsde/_brownian/brownian_tree.py#L175 and should have side='right' set, as np.searchsorted([0., 1.], 0.) == 0 for example.

searchsorted is used in a few places elsewhere and I'm not completely sure if they're correct either, which is why I'm opening an issue rather than just fixing it.

Incidentally torch.bucketize is available as of 1.6 so we can switch to that now.

lxuechen commented 3 years ago

Good point for searchsorted, I'll look closer into this. Thanks for mentioning bucketize, I'll try this.

lxuechen commented 3 years ago

Just checked, and I think you're right. This is the only place where I haven't checked that t is already in self._ts. When t is not in the list, side shouldn't matter. I'm sending in the fix and replacing np.searchsorted with bisect for performance.

patrick-kidger commented 3 years ago

Closed in #21.