QuantEcon / SimpleDifferentialOperators.jl

Library for simple upwind finite differences
MIT License
12 stars 2 forks source link

Fix Docs Mistakes #121

Closed arnavs closed 5 years ago

arnavs commented 5 years ago

PR to fix mistakes in documentation.

arnavs commented 5 years ago

@chiyahn I still want to give this another look, but wanted to post so as to avoid a bottleneck. If you think we are good, then when I'm done, we can push and retag.

codecov-io commented 5 years ago

Codecov Report

Merging #121 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #121   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         108    108           
=====================================
  Hits          108    108

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 8da91e4...e150fa9. Read the comment docs.

chiyahn commented 5 years ago

Codes look good to me. One more thing: you need to update the resulting plot as well (as implementation has slightly changed.)

arnavs commented 5 years ago

Roger. You mean this bit? plot(x, v, lw = 4, label = "v")

I think this OK, since v is evaluated on the interior nodes x, right? But feel free to push to this branch with fixes and merge whenever you think we're good.

chiyahn commented 5 years ago

No, I mean the implementation for $L_2$ is different so that the resulting plot for v is going to be different in the new version as well.

chiyahn commented 5 years ago

I'll push the new plots

arnavs commented 5 years ago

Oh, I see. You're including a static image. OK, will do.

Edit: Thanks, will let you push.

arnavs commented 5 years ago

We should probably also change this text, but I'll defer to you on that:

One can alternatively use differential operators on interior nodes and stack them with matrices for boundary conditions to compute v:

Since we're still passing in x\bar.

chiyahn commented 5 years ago

Oh, actually nevermind for plots. I can confirm that they return the same plots since it's being computed from extension operators rather than operators on interior nodes with bc.

We should probably also change this text, but I'll defer to you on that:

One can alternatively use differential operators on interior nodes and stack them with matrices for boundary conditions to compute v:

Since we're still passing in x\bar.

I agree, I'll make a fix and merge the PR

arnavs commented 5 years ago

Thanks @chiyahn, will leave this in your hands then.