Initial commit for the Black Scholes devito example #1288

rawsh · 2020-05-15T03:34:41Z

I've got a small notebook demonstrating using Devito to solve a PDE with nonzero boundary conditions, please take a look and let me know what you think. Thanks

review-notebook-app · 2020-05-15T03:34:47Z

Check out this pull request on

Review Jupyter notebook visual diffs & provide feedback on notebooks.

Powered by ReviewNB

codecov · 2020-05-15T03:38:52Z

Codecov Report

Merging #1288 into master will decrease coverage by 25.79%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #1288       +/-   ##
===========================================
- Coverage   86.64%   60.85%   -25.80%     
===========================================
  Files         182      147       -35     
  Lines       26153    17140     -9013     
  Branches     3598     3054      -544     
===========================================
- Hits        22661    10431    -12230     
- Misses       3064     6167     +3103     
- Partials      428      542      +114

Impacted Files	Coverage Δ
benchmarks/user/tools/__init__.py	`0.00% <0.00%> (-100.00%)`	⬇️
benchmarks/user/tools/executor.py	`0.00% <0.00%> (-83.79%)`	⬇️
devito/core/autotuning.py	`12.77% <0.00%> (-77.98%)`	⬇️
devito/data/utils.py	`18.34% <0.00%> (-74.24%)`	⬇️
devito/mpi/routines.py	`22.61% <0.00%> (-72.52%)`	⬇️
devito/finite_differences/coefficients.py	`18.34% <0.00%> (-70.45%)`	⬇️
benchmarks/user/tools/driver.py	`0.00% <0.00%> (-65.34%)`	⬇️
examples/checkpointing/checkpoint.py	`38.63% <0.00%> (-61.37%)`	⬇️
devito/builtins.py	`29.16% <0.00%> (-60.65%)`	⬇️
benchmarks/user/benchmark.py	`0.00% <0.00%> (-56.16%)`	⬇️
... and 122 more

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 e7e358e...1f1a02b. Read the comment docs.

rhodrin · 2020-05-15T12:22:41Z

@rawsh Thanks for this work and contribution. I'll take a good look through everything early next week.

FabioLuporini · 2020-05-15T12:23:19Z

from a quick glance this looks awesome.
I've just registered the reviewers for this PR (including myself). I will take a proper look soon.

Thanks a lot for this!

examples/finance/bs_ivbp.ipynb

mloubout · 2020-05-15T12:57:49Z

This looks super cool overall, thanks a lot for contributing it to the examples. I left a few minor comments but great work.

examples/finance/bs_ivbp.ipynb

georgebisbas · 2020-05-17T13:46:51Z

examples/finance/bs_ivbp.ipynb

@@ -0,0 +1,550 @@
+{


for i in range(shape[0]):
v.data[0, i] = max((smin + ds0 * i) - K, 0)
This can be done in one line right?

Reply via ReviewNB

I added another operator for the example without a boundary condition which added a line to the for loop so I left it how it was

georgebisbas

Thank you very much for this contribution Robert. I think the notebook is in a very good shape already. A few minor fixes may be needed, but overall great work. Congrats!

jkwashbourne

Nice work!

I still think you need a little more exposition for the boundary conditions, as that is the heart of the success here. I also think you might consider adding an example of what happens when there is no boundary condition. so my suggestions:

set up two operators, one without the BC and one with
run both ops and contrast the results
add more exposition for the BC, maybe some ASCII art as below, with comments like "x in the interior (s < smax) and determed by the PDE", "o in the boundary (s >= smax) and determined by the BC using a Eq" ...

ASCII ART

                      |  o  v(t,smax+1)  
                      | /
                      |/
                      o  v(t,smax)  
                     /|
                    / |
      v(t,smax-1)  X  |
                  /   | BC: constant slope for v when s >= smax
                 /    | for 4th order derivative, need 2 points 
   v(t,smax-2)  X     |   slope = v(t,smax-1) - v(t,smax-2)
               /      |   v(t,smax)   = v(t,smax-1) + slope
              /       |   v(t,smax+1) = v(t,smax)   + slope
v(t,smax-2)  X        |
                      |
     interior         |        boundary 
    (s < smax)      smax      (s >= smax)

jkwashbourne · 2020-05-18T02:39:24Z

Maybe you should also highlight how devito is stupid fast (your words) compared to the other solution. also ... needs units.

devito pde    timesteps:         2000,     0.029431 runtime
call_value_bs timesteps:            5,     3.802610 runtime

jkwashbourne · 2020-05-18T14:06:38Z

need to wrap the ASCII in three single quotes on blank lines both ends, per markdown code segment.

jkwashbourne · 2020-05-18T14:07:58Z

love the second example. suggest instead of "plateau", "fall incorrectly towards zero".

FabioLuporini · 2020-05-19T10:44:24Z

you may also wanna update the README here: https://github.com/devitocodes/devito/tree/master/examples#how-to-navigate-this-directory

with info about this new "finance" subdirectory

examples/finance/bs_ivbp.ipynb

rhodrin · 2020-05-19T17:47:12Z

examples/finance/bs_ivbp.ipynb

+    "smin = 70\n",
+    "smax = 130\n",
+    "\n",
+    "# If you want to try some different problems, uncomment these lines\n",


Would it be possible to add a short description/explanation re. the various choices of constants?

Robert you might add a link to the various papers we looked at to reproduce results.

Added an explanation. I added the papers I looked at that weren't behind paywalls (the 3 point approximation one I showed you is on research gate and I think the UNL math one is a better example anyway)

examples/finance/bs_ivbp.ipynb

rhodrin · 2020-05-19T18:04:06Z

Very nice notebook, thanks for the contribution. Some nitpicking above.

rawsh · 2020-05-20T22:04:19Z

Thank you for the comments! It was a fun notebook to work on.
@FabioLuporini I wasn't sure how to describe the finance directory but I added this for now:

finance: Jupyter notebooks with examples of applying Devito to partial differential equations with financial applications.

FabioLuporini · 2020-05-21T06:40:03Z

OK, I think this is ready, but we need to add it to our Continuous Integration framework. This means two small things:

Add an entry here, along the lines of what we do for the other tutorials: https://github.com/devitocodes/devito/blob/master/.github/workflows/tutorials.yml#L104
Add the #NBVAL_IGNORE_OUTPUT comment at the top of each cell producing potentially non-determininstic output (e.g., an Operator that runs and produces the string "Operator run in 0.01 sec" (the next time could be 0.02); a cell producing a plot). Take a look here -- several cells start with NBVAL_IGNORE_OUTPUT

@jkwashbourne I think you're already familiar with this

Initial commit for the Black Scholes devito example

b5d0685

add comment on boundary conditions

76c93d5

FabioLuporini assigned mloubout, rhodrin, georgebisbas, jkwashbourne and FabioLuporini and unassigned FabioLuporini, mloubout, rhodrin, georgebisbas and jkwashbourne May 15, 2020

FabioLuporini requested review from FabioLuporini, georgebisbas, jkwashbourne, mloubout and rhodrin May 15, 2020 12:20

mloubout reviewed May 15, 2020

View reviewed changes

examples/finance/bs_ivbp.ipynb Show resolved Hide resolved

examples/finance/bs_ivbp.ipynb Show resolved Hide resolved

examples/finance/bs_ivbp.ipynb Show resolved Hide resolved

examples/finance/bs_ivbp.ipynb Show resolved Hide resolved

add changes from pr

ae8a266

georgebisbas reviewed May 17, 2020

View reviewed changes

jkwashbourne requested changes May 17, 2020

View reviewed changes

rawsh added 2 commits May 17, 2020 23:13

add ascii, fix typos

6176a9f

add op for no boundary condition

54a4655

jkwashbourne approved these changes May 19, 2020

View reviewed changes

rhodrin reviewed May 19, 2020

View reviewed changes

examples/finance/bs_ivbp.ipynb Outdated Show resolved Hide resolved

rhodrin reviewed May 19, 2020

View reviewed changes

examples/finance/bs_ivbp.ipynb Outdated Show resolved Hide resolved

rhodrin reviewed May 19, 2020

View reviewed changes

examples/finance/bs_ivbp.ipynb Show resolved Hide resolved

rhodrin reviewed May 19, 2020

View reviewed changes

examples/finance/bs_ivbp.ipynb Outdated Show resolved Hide resolved

fix typos, change benchmark paragraph, add info in readme

1048049

add ignore outs and code coverage run

b0fa0fc

FabioLuporini approved these changes May 26, 2020

View reviewed changes

mloubout added 2 commits May 26, 2020 10:00

Merge branch 'master' into bs

1f1a02b

Merge branch 'master' into bs

6742692

mloubout merged commit 5d80838 into devitocodes:master May 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial commit for the Black Scholes devito example #1288

Initial commit for the Black Scholes devito example #1288

rawsh commented May 15, 2020

review-notebook-app bot commented May 15, 2020

codecov bot commented May 15, 2020 •

edited

rhodrin commented May 15, 2020

FabioLuporini commented May 15, 2020

mloubout commented May 15, 2020

georgebisbas May 17, 2020

rawsh May 20, 2020

georgebisbas left a comment

jkwashbourne left a comment •

edited

jkwashbourne commented May 18, 2020 •

edited

jkwashbourne commented May 18, 2020

jkwashbourne commented May 18, 2020

FabioLuporini commented May 19, 2020

rhodrin May 19, 2020

jkwashbourne May 19, 2020

rawsh May 20, 2020

rhodrin commented May 19, 2020

rawsh commented May 20, 2020

FabioLuporini commented May 21, 2020

Initial commit for the Black Scholes devito example #1288

Initial commit for the Black Scholes devito example #1288

Conversation

rawsh commented May 15, 2020

review-notebook-app bot commented May 15, 2020

codecov bot commented May 15, 2020 • edited

Codecov Report

rhodrin commented May 15, 2020

FabioLuporini commented May 15, 2020

mloubout commented May 15, 2020

georgebisbas May 17, 2020

Choose a reason for hiding this comment

rawsh May 20, 2020

Choose a reason for hiding this comment

georgebisbas left a comment

Choose a reason for hiding this comment

jkwashbourne left a comment • edited

Choose a reason for hiding this comment

jkwashbourne commented May 18, 2020 • edited

jkwashbourne commented May 18, 2020

jkwashbourne commented May 18, 2020

FabioLuporini commented May 19, 2020

rhodrin May 19, 2020

Choose a reason for hiding this comment

jkwashbourne May 19, 2020

Choose a reason for hiding this comment

rawsh May 20, 2020

Choose a reason for hiding this comment

rhodrin commented May 19, 2020

rawsh commented May 20, 2020

FabioLuporini commented May 21, 2020

codecov bot commented May 15, 2020 •

edited

jkwashbourne left a comment •

edited

jkwashbourne commented May 18, 2020 •

edited