devito
devito copied to clipboard
Examples: Add Darcy flow example
This PR adds a notebook showing an example of mapping permeability fields to pressure flow using the Darcy flow equation, which is a single-phase porous media flow equation.
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Thanks @ sashaowen for the verynice addition to the tutorials. I left a few comments but it shouldn't be too much to fix.
We usually run all the tutorials through the CI, so it would be good to
- FIx the random seed so that the result is always the same
- Add an assert on the norm of the result so that we know it is always running correctly
- add the
#NBVAL_IGNORE_OUTPUT
to cells generating figures or such to avoid CI errors.
let us know if you need some help with it.
Hi, thanks for your replies, I have made the following changes:
- converted the GRF class to numpy and removed torch from imports
- Included three samples instead of one
- fixed the equation
- fixed a random seed
- added an assert on the norm of the results
- added #NBVAL_IGNORE_OUTPUT to the cells generating figures Thanks!
Thanks @georgebisbas, the latest commit should have the changes mentioned in your comments :-)
Left some extra minor comments but is almost there, thanks for the patience.
Please rebase on top of master and try to follow commit message guidelines: https://github.com/devitocodes/devito/wiki/Tags-for-commit-messages-and-PR-titles
Left some extra minor comments but is almost there, thanks for the patience.
Please rebase on top of master and try to follow commit message guidelines: https://github.com/devitocodes/devito/wiki/Tags-for-commit-messages-and-PR-titles
Thanks for your comments!, Hopefully I have followed the commit guidelines properly this time, apologies for the previous ones. I have made the changes mentioned in your comments, however I'm unsure what rebase on top of master means, could you please point me to a resource on how to do it? Thanks!
Sorry for the delay. rebase
lets you rewrite your commit history and make sure that your branch is up to date with master and is mergeable.
You can simply run git rebase -i origin/master
(ou'll need your fork to be up to date on master) and it will pop up an editable message with guidelines on what to do.
Hi @mloubout I think I have addressed your comments, would you mind taking another look when you get the chance please? Hopefully I have done it right!
Hi. It looks good now. Your branch is out of date with master and need to be rebased and will be good to go https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase#:~:text=What%20is%20git%20rebase%3F,of%20a%20feature%20branching%20workflow.
Codecov Report
Merging #1998 (88bb3c7) into master (88bb3c7) will not change coverage. The diff coverage is
n/a
.
:exclamation: Current head 88bb3c7 differs from pull request most recent head d2706a5. Consider uploading reports for the commit d2706a5 to get more accurate results
@@ Coverage Diff @@
## master #1998 +/- ##
=======================================
Coverage 87.77% 87.77%
=======================================
Files 214 214
Lines 36984 36984
Branches 5570 5570
=======================================
Hits 32464 32464
Misses 4001 4001
Partials 519 519
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Hi. It looks good now. Your branch is out of date with master and need to be rebased and will be good to go https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase#:~:text=What%20is%20git%20rebase%3F,of%20a%20feature%20branching%20workflow.
I think this is up to date now? + rebased, I think, thankyou for your patience!!
Thanks, all good indeed. Looks liek the assert is failing in the notebook though
----> 1 assert np.isclose(LA.norm(output1),1.0335084)
2 assert np.isclose(LA.norm(output2),1.3038709)
3 assert np.isclose(LA.norm(output3),1.3940924)
Maybe a tolerance would make it more stable , for example np.isclose(a, b, atol=1e-4, rtol=0)
should be ok usually
@mloubout Added a tolerance and seemed to run fine locally so hopefully that should work now :)
@mloubout still appear to be errors with the assert, but I'm not sure why this is, when I run it in Vs code from github desktop it works?
I just ran the same code separately in a jupyter server compared to VS code and got slightly different numbers, i.e
output 1 in VS code: 1.0335084
output 1 in a Jupyter server: 1.0335636
running the assert in the Jupyter server failed for 1e-4 but worked for 1e-3, I could try changing to 1e-3 if that is acceptable?
Yes
committed with 1e-3.