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

Migrate functionality to HighDimPDE.jl #514

Closed
7 tasks
ChrisRackauckas opened this issue May 13, 2022 · 10 comments
Closed
7 tasks

Migrate functionality to HighDimPDE.jl #514

ChrisRackauckas opened this issue May 13, 2022 · 10 comments

Comments

@ChrisRackauckas
Copy link
Member

ChrisRackauckas commented May 13, 2022

@vboussange
Copy link

The migration of NNPDENS and the associated test file is addressed in the specific issue SciML/HighDimPDE.jl#30.

@vboussange
Copy link

vboussange commented Jun 10, 2022

Work in progress on branch https://github.com/SciML/HighDimPDE.jl/tree/NeuralPDE_integration.

Thanks to @ashutosh-b-b, the integration is almost completed and partially works. A major change is that now in HighDimPDE.jl, only one algorithm, DeepBSDE, replaces the former NNPDENS and NNPDEHAN from NeuralPDE.jl. The latter were basically the same algorithms, except that NNPDENS was used to call a solve function that would allow the use of SDE solvers from DifferentialEquations.jl to solve the reformulated stochastic problem. What happens now is that if an sdealg is provided in the solve function, then a different solve function which formulates the SDE problem is called (see : https://github.com/SciML/HighDimPDE.jl/blob/a83198b733f94865824f7d6dfa29188077ccebd5/src/DeepBSDE.jl#L72 ).

What still needs to be done:

  • Clean up the tests, located in test/DeepBSDE_Han.jl and test/DeepBSDE_Han.jl. Style should be similar to e.g. test/DeepSplitting.jl.
  • Fix the tests that are not working (marked by a #TODO comment).
  • Add a page on the documentation under solver algorithms, documenting DeepBSDE.
  • Migrate the tutorials listed above.

@ChrisRackauckas
Copy link
Member Author

@ashutosh-b-b you had a PR around this? What's left here?

@ashutosh-b-b
Copy link
Contributor

Its merged. HighDimPDE#32, I guess mostly the documentation is left there.

@vboussange
Copy link

@ChrisRackauckas,@ashutosh-b-b, the PR is merged, but on the branch https://github.com/SciML/HighDimPDE.jl/tree/NeuralPDE_integration and not on the main branch, as stated above.

@ashutosh-b-b, Could you take look at the test files DeepBSDE_Han.jl and DeepBSDE.jl, and fix the tests with annotated #TODO? On my side, I will complete the documentation, and migrate the tutorials.

@ashutosh-b-b
Copy link
Contributor

@ChrisRackauckas @vboussange the problem with some of the tests is ambiguity regarding methods on Tracked variables. For example:
MethodError: vcat(::TrackedArray{…,Vector{Float32}}, ::Vector{Tracker.TrackedReal{Float32}}) is ambiguous

is what we get for DeepBSDE - Nonlinear Black-Scholes Equation with Default Risk

and

MethodError: no method matching Float32(::Tracker.TrackedReal{Float32}) for
DeepBSDE - Black-Scholes-Barenblatt equation

@ChrisRackauckas
Copy link
Member Author

If you end up with Array{TrackedReal} you'll take quite a performance hit. Are you using any mutation that would trigger this?

@ashutosh-b-b
Copy link
Contributor

Yeap, its here. and here

@ChrisRackauckas
Copy link
Member Author

That's not mutation, but would still trigger it. Maybe do Tracker.collect(vcat(σ(X,_p,t),_σᵀ∇u))?

@vboussange
Copy link

@ChrisRackauckas @vboussange the problem with some of the tests is ambiguity regarding methods on Tracked variables. For example: MethodError: vcat(::TrackedArray{…,Vector{Float32}}, ::Vector{Tracker.TrackedReal{Float32}}) is ambiguous

is what we get for DeepBSDE - Nonlinear Black-Scholes Equation with Default Risk

and

MethodError: no method matching Float32(::Tracker.TrackedReal{Float32}) for DeepBSDE - Black-Scholes-Barenblatt equation

@ashutosh-b-b @ChrisRackauckas , where are we now with this? Would be nice to finally merge all of this work and make a release, since NeuralPDE.jl does not provide anymore the algorithms. Why do we use Tracker here? Wouldn't Zygote be more appropriate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants