Added solution plot function #16
Conversation
Add solution plots to poisson solver notebook
|
@yguclu please review this pr :) |
| "metadata": { | ||
| "kernelspec": { | ||
| "display_name": "Python 3 (ipykernel)", | ||
| "language": "python", | ||
| "name": "python3" | ||
| }, | ||
| "language_info": { | ||
| "codemirror_mode": { | ||
| "name": "ipython", | ||
| "version": 3 | ||
| }, | ||
| "file_extension": ".py", | ||
| "mimetype": "text/x-python", | ||
| "name": "python", | ||
| "nbconvert_exporter": "python", | ||
| "pygments_lexer": "ipython3", | ||
| "version": "3.10.12" | ||
| } | ||
| }, |
There was a problem hiding this comment.
It seems to me that this metadata can/should be cleaned before a commit. @yguclu do you agree?
There was a problem hiding this comment.
I am no Jupyter Notebook expert, but I think so, yes.
There was a problem hiding this comment.
Not Yaman but I agree @campospinto. The metadata here is hardcoding Python version info which we're trying to avoid. @anushkasinghh please clean the notebook as stated in the guide:
# Clean notebook
nb-clean clean --remove-empty-cells --remove-all-notebook-metadata chapter1/poisson.ipynb
# Check if notebook is clean. No output means the notebook is already clean (otherwise you'll get an error)
nb-clean check chapter1/poisson.ipynbThere was a problem hiding this comment.
@kvrigor I have noticed the cell IDs being changed in this PR. Is this something unavoidable, or we could keep the original values somehow?
There was a problem hiding this comment.
My surface reading of this doc suggests that cell IDs should change whenever cells change, since the primary purpose of a cell ID is to uniquely identify cell contents. Unfortunately we can't avoid mutating cell IDs, and maybe that's one of the reasons why a Jupyter notebook isn't version-control friendly.
There was a problem hiding this comment.
Is a cell ID something like a hash code? It seems to me that a new one could be generated when opening the notebook, rather than hardcoded in the file...
There was a problem hiding this comment.
Is a cell ID something like a hash code?
It's possible; hardcoding it might be necessary for efficiency and also for scripting access by IDE tools.
| "\n", | ||
| "domain = Square()\n", | ||
| "from sympy import pi, sin, lambdify\n", |
There was a problem hiding this comment.
git sees some changes because this import now comes after the line break. If you keep the same line breaks than in the original code, git will only see the relevant changes and the commit will be a bit cleaner.
| "\n", | ||
| "V = ScalarFunctionSpace('V', domain)\n", | ||
| "domain = Square()\n", |
There was a problem hiding this comment.
same here: the only changes here are the position of the line breaks "\n",
| " num = np.array([[uh(e1, e2) for e2 in eta2] for e1 in eta1])\n", | ||
| "\n", | ||
| " # Compute exact solution\n", | ||
| " ex = np.array([[phi_exact(e1, e2) for e2 in eta2] for e1 in eta1])\n", |
There was a problem hiding this comment.
it seems to me that this line triggers an error when the notebook is executed. @anushkasinghh did you verify that you could run the notebooks ?
When the CI tests are run with the PR (probably after each new commit) you can also check that the "book" (which will be deployed after this PR is merged) looks as it should, by downloading the artifacts as described in the readme.
But you should first check that you can run the notebook correctly before committing, that will be simpler :)
|
closing as issues resolved in another pr |
No description provided.