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

Multi output surrogates #106

Merged
merged 9 commits into from
Dec 13, 2019
Merged

Conversation

platawiec
Copy link
Contributor

Ref #98. Multi-output surrogates have been created for the following, and AD is confirmed to work as well:

  • NeuralSurrogate
  • InverseDistanceSurrogate
  • SecondOrderPolynomialSurrogate
  • RadialBasis

This PR is not planned to implement multi-output surrogate optimization. The only other surrogate which is interesting to implement is Kriging, but I will save that for a PR where I implement a Stheno back-end. That PR will also address #100 for Kriging.

This PR slightly changes the SecondOrderPolynomialSurrogate implementation to include all n choose 2 combinations of input dimensions.

This PR preserves the type of the flux model output. In other words, for a user to return a scalar from their ANN, they should write: model = Chain(Dense(2,1), sum) to explicitly reduce the model output to a scalar.

I currently don't plan on implementing the other surrogates (Lobachesky, etc.) as multi-output, but if there's interest I will. The rest follow the same pattern where each output is fit separately (at least to the extent I'm aware of). Those implementations would be fairly boilerplate, I believe.

@platawiec
Copy link
Contributor Author

Hmm, I'm currently seeing LoadError: ArgumentError: Cannot setindex! to -0.6390356103155185 for an AbstractFill with value -63.903561031551845.. I previously saw this when defining my own models in Flux v0.10, so I switched to Flux 0.9 and didn't have any issues. I'm not sure why it's cropping up here, and I'm also not sure how to address the root cause.

@ChrisRackauckas
Copy link
Member

I previously saw this when defining my own models in Flux v0.10, so I switched to Flux 0.9 and didn't have any issues.

The fill vector probably got concretized on v0.9. I'd just make sure you explicitly concretize it.

@ludoro
Copy link
Contributor

ludoro commented Dec 13, 2019

Oh wow, amazing work! When the PR will be ready to be merged I will open a good first issue for the implementation of the remaining easier multi out surrogates

@platawiec
Copy link
Contributor Author

Ok, replacing the explicit scalar output of flux models with Chain(Dense(2,1), first) instead of Chain(Dense(2,1), sum) makes this work on my setup for Flux v0.10

@ChrisRackauckas
Copy link
Member

Awesome! Let's see if tests pass.

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #106 into master will decrease coverage by 0.58%.
The diff coverage is 96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   90.65%   90.06%   -0.59%     
==========================================
  Files          12       13       +1     
  Lines        1081     1037      -44     
==========================================
- Hits          980      934      -46     
- Misses        101      103       +2
Impacted Files Coverage Δ
src/Surrogates.jl 100% <ø> (+16.66%) ⬆️
src/NeuralSurrogate.jl 100% <100%> (ø) ⬆️
src/InverseDistanceSurrogate.jl 100% <100%> (ø) ⬆️
src/utils.jl 33.33% <33.33%> (ø)
src/SecondOrderPolynomialSurrogate.jl 97.29% <96.96%> (-2.71%) ⬇️
src/Radials.jl 98.59% <97.56%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4c0a6f...e907fec. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #106 into master will decrease coverage by 0.58%.
The diff coverage is 96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   90.65%   90.06%   -0.59%     
==========================================
  Files          12       13       +1     
  Lines        1081     1037      -44     
==========================================
- Hits          980      934      -46     
- Misses        101      103       +2
Impacted Files Coverage Δ
src/Surrogates.jl 100% <ø> (+16.66%) ⬆️
src/NeuralSurrogate.jl 100% <100%> (ø) ⬆️
src/InverseDistanceSurrogate.jl 100% <100%> (ø) ⬆️
src/utils.jl 33.33% <33.33%> (ø)
src/SecondOrderPolynomialSurrogate.jl 97.29% <96.96%> (-2.71%) ⬇️
src/Radials.jl 98.59% <97.56%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4c0a6f...e907fec. Read the comment docs.

@ChrisRackauckas ChrisRackauckas merged commit 74243dc into SciML:master Dec 13, 2019
@ChrisRackauckas
Copy link
Member

Nicely done.

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 this pull request may close these issues.

3 participants