google-research / torchsde

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

Euler-Heun method #39

Closed mtsokol closed 4 years ago

mtsokol commented 4 years ago

Hi! Just euler-heun method. Tried to run it but it hangs on BrownianInterval so related to #36

mtsokol commented 4 years ago

@patrick-kidger Hi! Just rebased - now it all works:

mtsokol commented 4 years ago

@patrick-kidger I've rewritten stratonovich diagnostics to scalar noise type (found one issue). Should I commit those files also? (it's basically copy paste of diagonal except for problem) Or only post plots here? (and the same question regarding additive noise type)

patrick-kidger commented 4 years ago

As you've got them, let's include the files as well. We'll probably be interested in running those later too. (In fact the diagnostics all share a lot of common code that should be factored out somewhere, but that's not super important now.)

mtsokol commented 4 years ago

Ok, I included stratonovich diagnostics for additive and scalar. For both basic plots look OK.

Rates plots look like this:

There's an issue with milstein solver - for scalar noise dimensions does not conform to compute Y_prime as g it's one additional dimension bigger (included fix but it's a quick one just to make it pass).

And looking at rates milstein for additive it's wrong but haven't figured it out yet. Btw both grad-free and default use gdg_prod_default - according to comment grad_free path isn't ready yet, right?

lxuechen commented 4 years ago

So for additive noise SDEs, the diffusion doesn't depend on the state. So the Ito interpretation is the same as the Strat interpretation, and the Milstein schemes should all have the same strong order as the Euler scheme.

mtsokol commented 4 years ago

Thanks for review - now I got it. Updated plots above - now it all looks right.

patrick-kidger commented 4 years ago

This LGTM. @lxuechen happy for me to pull the squash+merge trigger?

lxuechen commented 4 years ago

The code in general is okay. I'd recommend using a linter with the character limits and blank line between imports/functions enforced.

Thanks again for the putting in the effort in implementing these methods.

mtsokol commented 4 years ago

Thanks for review! My main PL is Scala and python integration in my VS code barely works but I will setup that linter options to avoid those comments in the future.

mtsokol commented 4 years ago

Just one moment - milstein naming should be fixed also in diagonal and scalar files.

mtsokol commented 4 years ago

@lxuechen OK, now it's ready.

mtsokol commented 4 years ago

P.S. (asking out of curiosity) Do you have a roadmap for merging dev into master and cutting new release?

patrick-kidger commented 4 years ago

@mtsokol Loosely speaking yes, although @lxuechen and I haven't fixed things in detail.

I think we'd like to tick off the remaining things in #15. Fortunately, I think we've now done all the complicated ones, and it's mostly just grunt work remaining. My hope is for a new release this month. (I'm only speaking personally there; Chen and I haven't discussed release schedules yet.)

lxuechen commented 4 years ago

@mtsokol @patrick-kidger

I agree mostly with Patrick. Aside from the technical problems in #15, on a high level, I think the most crucial missing things are

I think we should be able to pull off another release in this month, if things go well. Though, I also think we shouldn't rush things by too much and only merge when we're genuinely ready.