Issue 312 tutorial #814
Issue 312 tutorial #814
Conversation
|
How does this play with the "first steps"? |
I think the purpose is a bit different. The tutorials are taking it very slowly and go through a lot of details from the ground up (in theory, assuming they're done), while the "first steps" is more of a showcase example (maybe that would be the better title for it). In any case, the text there would need some update I guess. |
There was a problem hiding this comment.
Great addition overall, really like it and it is sorely needed.
Anyway:
-
Should we port the old "first steps" to this format as well? it fits rather nicely.
-
Are these automatically run by some test? i saw some error that should give an Exception.
Anyway this review is not 100% done, I'll look through the text again.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "The side lenghts of the rectangle are $2\\pi / s$, where $s = 0.01$ is the pixel side length of the real space grid. Note that the rectangle is slightly shifted into the negative direction (this can be controlled with a parameter)." |
There was a problem hiding this comment.
Formatting of this is ugly (the raw code, not the output). Anyway, what parameter?
| "source": [ | ||
| "% matplotlib inline\n", | ||
| "phantom = odl.util.phantom.shepp_logan(l2_discr, modified=True) # Shepp-Logan phantom in the discretized space\n", | ||
| "fig = phantom.show('The famous Shepp-Logan phantom') # Storing the figure so we don't get two inline plots" |
There was a problem hiding this comment.
use a semicolon instead to stop the printing of a second figure, then you don't need a comment (imo).
i.e
phantom.show('The famous Shepp-Logan phantom');
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "X = odl.Rn(3)\n", |
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "A = np.array([[1.0, 2.0, 0.0],\n", |
There was a problem hiding this comment.
No real reason to make this an array, just go with a list?
| ], | ||
| "source": [ | ||
| "x = X.element([1, 1, 1])\n", | ||
| "A_op(x)" |
There was a problem hiding this comment.
perhaps also show of directly calling with a list and mention that it does magic.
| # part of that expression, of course prepending the package name and a | ||
| # dot, a trick we know from NumPy: | ||
|
|
||
| odl.fn(3, 'int') |
There was a problem hiding this comment.
dtypes should not come in the first example, change to floats and pretend it doesn't happen.
There was a problem hiding this comment.
I know what you mean, but honestly who wouldn't use [1, 2, 3] as argument to vector? I didn't see a good way around this.
What pedagogical mistake should we do:
- use an input that most users wouldn't have thought of or
- have too much complexity in the first example?
There was a problem hiding this comment.
Well the main mistake we do here is using odl.vector which has a weird default since numpy has a weird default. With that said, I'd quickly leave odl.vector territory (and be clear that we start wiht spaces, not vectors) and then we're on somewhat solid ground. This example is obviously aimed at quite low level people so I'd try to keep dtypes and stuff out.
| # come back when you're finished! | ||
|
|
||
| # We import this for Python 2 compatibility | ||
| from __future__ import print_function |
There was a problem hiding this comment.
no real need, just put parenthesis around and you're fine (unless you use some arguments to the function or use print as a first class object, which I guess you dont).
| # *vector*. If you're familiar with [NumPy](http://www.numpy.org/), you | ||
| # can simply think of a `numpy.ndarray`. | ||
|
|
||
| x = odl.vector([1, 2, 3]) |
There was a problem hiding this comment.
I think using odl.vector in a first example sets a bad predecent, but I see why you do it. Have you considered starting with a space?
There was a problem hiding this comment.
Would be an alternative. But really, if you know nothing about ODL and are not necessarily a math guy, what would be the easier first step?
There was a problem hiding this comment.
Well there is also the trade-off of them simply learning something that is "worthless" in a sense. I truly don't know if a user that knows that little (i.e. first year university math) could be expected to use ODL to any extent anyway. But as long as you stick to spaces after the first few lines I guess we're fine.
| s.field | ||
|
|
||
|
|
||
| s.size # the equivalent of "length", NumPy users will recognize it |
There was a problem hiding this comment.
No, it is not equivalent. len gives the maximum value of the first index, size the total size, i.e. the product of the maximum lengths of all indices. I guess any reader would be fine, but lets try to be strict. Simply say "the number of elements in the space" or something.
| @@ -0,0 +1,14 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Strongly against platform specific stuff like .sh, why not make this script in python?
There was a problem hiding this comment.
If we decide to let Travis take care of the Python script generation, bash is fine.
There was a problem hiding this comment.
Well then we cant generate them locally on windows or mac. I felt like the whole idea with python etc is that we stay platform independent, even for developers.
|
Overall comment: Why are the ipython notebooks even checked into the repo? Shouldnt we only have the source code here and put the notebooks themselves into the gitignore? That's how we handle the rest of the doc. |
|
Don't look too much at the stubs. I just collected them somewhere for later use. They're nowhere near useful right now and may contain errors. But they're at least half thought-out and may come in handy when someone writes a proper tutorial of those topics.
Why? Regarding notebooks vs. Python codeCurrently, the notebooks are the main files. I don't see a reason not to have them in the repo, it's just JSON after all. Outputs should be cleared though, they may contain images. As I wrote above, the Python scripts are generated from the notebooks, so we could leave them out. The only thing that's not possible is to leave out the notebooks :-) |
Absolutely no idea, i clicked update branch but that's never done a merge for me. |
Well at least one should be left out, I strongly dislike duplication. Regarding Ipython, is this the only format for Ipython notebooks? It is frankly not really readable and I especially dislike the fact that they contain the results as well as the calls, i.e. if we change something in ODL (like a repr) we'll have to change them as well. |
Yeah that's the only format. Actually, in GitHub when you click on a notebook you should get a rendered version. But often it doesn't work. Alternatively you can go to https://nbviewer.jupyter.org and paste a link to a notebook. For example the basic one. Regarding output, that's only because I forgot to clean the output. It's only stored if you save while your output is still there. Better to distribute notebooks with empty output. |
| - Python script | ||
| - Level | ||
|
|
||
| * - :download:`ODL Basics.ipynb <notebooks/ODL Basics.ipynb>` |
There was a problem hiding this comment.
Why can't they explore them online instead? It feels like this whole installation guide is massively overcomplicated for users who apparently barely know what python is.
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "from __future__ import print_function # For Python2 compatibility\n", |
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "The spaces with real or complex floating point data types have own constructors `rn` and `cn`, mostly because they are most heavily used. Mathematically, they stand for $n$-dimensional Euclidean spaces $\\mathbb{R}^n$ or $\\mathbb{C}^n$, respectively, where in our case, $n = 3$. In general, we usually write $\\mathbb{F}$ for \"$\\mathbb{R}$ or $\\mathbb{C}$\" and $\\mathbb{F}^n$ for the $n$-dimensional Euclidean vector space over the *field* $\\mathbb{F}$.\n", |
There was a problem hiding this comment.
| " \\langle\\cdot, \\cdot\\rangle : \\mathbb{R}^n \\times \\mathbb{R}^n \\to \\mathbb{R}.\n", | ||
| "$$\n", | ||
| "\n", | ||
| "It is *linear in the first argument*, *conjuagte-symmetric* and *positive definite*. Check [here](https://en.wikipedia.org/wiki/Inner_product_space#Definition) for the exact definitions. The standard inner product on $\\mathbb{R}^n$ is also known as \"dot product\" and defined as\n", |
There was a problem hiding this comment.
conjuagte-symmetric makes little sense on Rn, also spelling.
| "\n", | ||
| "cos_angle_rad = x.inner(y) / np.sqrt(x.inner(x) * y.inner(y))\n", | ||
| "angle_rad = np.arccos(cos_angle_rad)\n", | ||
| "angle_deg = angle_rad * RAD2DEG\n", |
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "In fact, the default implementation of a `vector` is just a thin wrapper around `numpy.ndarray`. However, this back-end can be switched for something else, e.g., a CUDA-based data container. On the surface it will look exactly the same, and the differences will be entirely under the hood. More on that later.\n", |
There was a problem hiding this comment.
How to swap? At least mention it now that you have teased them, or make clickable.
| "metadata": { | ||
| "collapsed": false | ||
| }, | ||
| "outputs": [ |
There was a problem hiding this comment.
On outputs: You could proably parse these programatically and remove all outputs sections.
| } | ||
| ], | ||
| "source": [ | ||
| "print(s.__doc__)" |
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "s = odl.fn(3, 'int')" |
There was a problem hiding this comment.
is "s" really the naming convention we want to teach people?
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "Also the word \"contains\" resonates with experienced Python users. They think of the `__contains__` method and want to probe it with the `object in container` expression. Here, the vector should be in the space:" |
There was a problem hiding this comment.
Way to much words, simply call x in s without the long introduction. I guess you can keep "Here, the vector should be in the space:"
|
Status on this? |
|
I can't spend time on this now, needs to wait. |
|
Any chance this could get some love? Would be freaking great |
|
Once again, do we have a plan for this, or do we refer people to the odlworkshop stuff? |
Initial effort to add some really basic tutorials to ODL. Looks like a big PR but really isn't. It consists in