-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve documentation #598
base: main
Are you sure you want to change the base?
Conversation
📚 Documentation |
These three references should also be changed into something like [KABS], [WAHB], [MAG2018] mrpro/src/mrpro/data/Rotation.py Line 1824 in 4acaff1
|
"use github style for code examples (not sure about this)" |
But even for the github style the background should depend on the browser settings, shouldn't it? |
a8f7574
to
e395218
Compare
a94e9b2
to
e00b6a7
Compare
b1bcec2
to
6606672
Compare
commit 399ea3b Author: Felix Zimmermann <[email protected]> Date: Thu Jan 9 19:35:48 2025 +0100 notebools commit 9a24e28 Author: Felix Zimmermann <[email protected]> Date: Thu Jan 9 17:18:08 2025 +0100 changes to leonids pr commit 13c4e7e Author: lrlunin <[email protected]> Date: Wed Dec 18 16:24:36 2024 +0100 fix notebooks_path output commit 93ef986 Author: lrlunin <[email protected]> Date: Wed Dec 18 16:21:23 2024 +0100 fix matrix element name commit 7eccf87 Merge: 2d3c3ff 9dc1167 Author: Lunin Leonid <[email protected]> Date: Wed Dec 18 16:14:30 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit 2d3c3ff Author: lrlunin <[email protected]> Date: Wed Dec 18 15:46:10 2024 +0100 remove space after markdown for colab badge commit 9a5556d Author: lrlunin <[email protected]> Date: Wed Dec 18 15:45:25 2024 +0100 add space after markdown for colab badge commit fba5a6f Author: lrlunin <[email protected]> Date: Wed Dec 18 15:05:55 2024 +0100 add colab badge for each notebook commit 159afed Author: lrlunin <[email protected]> Date: Wed Dec 18 14:17:45 2024 +0100 update jupytext version commit 34d43f0 Author: lrlunin <[email protected]> Date: Tue Dec 10 13:58:43 2024 +0100 use find for notebook listing in docs commit f76b236 Author: lrlunin <[email protected]> Date: Tue Dec 10 13:23:29 2024 +0100 one-way conversion from .py to .ipynb, remove preamble from .py representation commit 865347a Author: lrlunin <[email protected]> Date: Wed Nov 20 22:45:42 2024 +0100 fixed path for notebooks in examples commit 1688c70 Author: lrlunin <[email protected]> Date: Wed Nov 20 22:44:16 2024 +0100 fixed trigger for jupytext commit b9b3a6f Author: lrlunin <[email protected]> Date: Wed Nov 20 22:40:17 2024 +0100 split scripts and notebooks commit 279a578 Merge: beddf13 8d24ebb Author: Lunin Leonid <[email protected]> Date: Tue Nov 19 21:10:30 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit beddf13 Author: lrlunin <[email protected]> Date: Tue Nov 19 21:10:03 2024 +0100 sync .ipynb/.py in pre-commit, add preamble in .py files commit 0600a53 Author: lrlunin <[email protected]> Date: Tue Nov 19 18:42:47 2024 +0100 moved examples ruff config to examples folder, removed verbose from pre-commit hook commit 36f3b7a Author: lrlunin <[email protected]> Date: Tue Nov 19 18:25:57 2024 +0100 fixed missing kernelspec commit 4d610c6 Author: lrlunin <[email protected]> Date: Fri Nov 15 14:23:38 2024 +0100 also removing metadata.language_info commit 48e8080 Author: lrlunin <[email protected]> Date: Thu Nov 14 22:45:35 2024 +0100 clean kernel related information from the cells commit f8aa621 Author: lrlunin <[email protected]> Date: Thu Nov 14 20:44:13 2024 +0100 remove mention of convert steps commit 534aaa0 Author: lrlunin <[email protected]> Date: Thu Nov 14 20:37:43 2024 +0100 reset cell id commit 9f5f2da Merge: 74675d6 38722bf Author: Lunin Leonid <[email protected]> Date: Thu Nov 14 20:36:21 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit 74675d6 Merge: ae3a613 c268ad2 Author: Lunin Leonid <[email protected]> Date: Sun Nov 10 20:50:44 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit ae3a613 Author: lrlunin <[email protected]> Date: Sun Nov 10 20:50:10 2024 +0100 reset the cells to init states commit 5d1dba2 Author: lrlunin <[email protected]> Date: Fri Nov 8 16:43:47 2024 +0100 moved notebooks formatting and update to pre-commit
402e8c5
to
a92877b
Compare
965a743
to
47ad68e
Compare
commit 36785c4 Author: lrlunin <[email protected]> Date: Fri Jan 10 13:57:38 2025 +0100 move cartesion_reconstruction in correct folders commit d3b0d1b Author: lrlunin <[email protected]> Date: Fri Jan 10 13:54:34 2025 +0100 run mypy hook as last commit b66b9b7 Author: Lunin Leonid <[email protected]> Date: Thu Jan 9 22:22:38 2025 +0100 fix notebook_path variable in matrix commit 8e2cb8e Merge: f450529 a1873a0 Author: Lunin Leonid <[email protected]> Date: Thu Jan 9 22:20:50 2025 +0100 Merge branch 'main' into notebooks-in-pre-commit commit f450529 Author: Felix F Zimmermann <[email protected]> Date: Thu Jan 9 22:15:20 2025 +0100 changes to leonids pr (#602) commit a1873a0 Author: Lunin Leonid <[email protected]> Date: Wed Jan 8 09:54:12 2025 +0100 Add dark and light logo to README (#600) commit f040c00 Author: Felix F Zimmermann <[email protected]> Date: Wed Jan 8 00:07:27 2025 +0100 Release v0.250107 (#599) commit 13c4e7e Author: lrlunin <[email protected]> Date: Wed Dec 18 16:24:36 2024 +0100 fix notebooks_path output commit 93ef986 Author: lrlunin <[email protected]> Date: Wed Dec 18 16:21:23 2024 +0100 fix matrix element name commit 7eccf87 Merge: 2d3c3ff 9dc1167 Author: Lunin Leonid <[email protected]> Date: Wed Dec 18 16:14:30 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit 2d3c3ff Author: lrlunin <[email protected]> Date: Wed Dec 18 15:46:10 2024 +0100 remove space after markdown for colab badge commit 9a5556d Author: lrlunin <[email protected]> Date: Wed Dec 18 15:45:25 2024 +0100 add space after markdown for colab badge commit fba5a6f Author: lrlunin <[email protected]> Date: Wed Dec 18 15:05:55 2024 +0100 add colab badge for each notebook commit 159afed Author: lrlunin <[email protected]> Date: Wed Dec 18 14:17:45 2024 +0100 update jupytext version commit 34d43f0 Author: lrlunin <[email protected]> Date: Tue Dec 10 13:58:43 2024 +0100 use find for notebook listing in docs commit f76b236 Author: lrlunin <[email protected]> Date: Tue Dec 10 13:23:29 2024 +0100 one-way conversion from .py to .ipynb, remove preamble from .py representation commit 865347a Author: lrlunin <[email protected]> Date: Wed Nov 20 22:45:42 2024 +0100 fixed path for notebooks in examples commit 1688c70 Author: lrlunin <[email protected]> Date: Wed Nov 20 22:44:16 2024 +0100 fixed trigger for jupytext commit b9b3a6f Author: lrlunin <[email protected]> Date: Wed Nov 20 22:40:17 2024 +0100 split scripts and notebooks commit 279a578 Merge: beddf13 8d24ebb Author: Lunin Leonid <[email protected]> Date: Tue Nov 19 21:10:30 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit beddf13 Author: lrlunin <[email protected]> Date: Tue Nov 19 21:10:03 2024 +0100 sync .ipynb/.py in pre-commit, add preamble in .py files commit 0600a53 Author: lrlunin <[email protected]> Date: Tue Nov 19 18:42:47 2024 +0100 moved examples ruff config to examples folder, removed verbose from pre-commit hook commit 36f3b7a Author: lrlunin <[email protected]> Date: Tue Nov 19 18:25:57 2024 +0100 fixed missing kernelspec commit 4d610c6 Author: lrlunin <[email protected]> Date: Fri Nov 15 14:23:38 2024 +0100 also removing metadata.language_info commit 48e8080 Author: lrlunin <[email protected]> Date: Thu Nov 14 22:45:35 2024 +0100 clean kernel related information from the cells commit f8aa621 Author: lrlunin <[email protected]> Date: Thu Nov 14 20:44:13 2024 +0100 remove mention of convert steps commit 534aaa0 Author: lrlunin <[email protected]> Date: Thu Nov 14 20:37:43 2024 +0100 reset cell id commit 9f5f2da Merge: 74675d6 38722bf Author: Lunin Leonid <[email protected]> Date: Thu Nov 14 20:36:21 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit 74675d6 Merge: ae3a613 c268ad2 Author: Lunin Leonid <[email protected]> Date: Sun Nov 10 20:50:44 2024 +0100 Merge branch 'main' into notebooks-in-pre-commit commit ae3a613 Author: lrlunin <[email protected]> Date: Sun Nov 10 20:50:10 2024 +0100 reset the cells to init states commit 5d1dba2 Author: lrlunin <[email protected]> Date: Fri Nov 8 16:43:47 2024 +0100 moved notebooks formatting and update to pre-commit
docs/source/api.rst
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a short intro and the docstrings to modules, making the subsection redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ruff formatted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed some of the comments from the tutorial.
] | ||
|
||
|
||
intersphinx_mapping = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes torch.Tensor etc clickable links
default_role = 'py:obj' | ||
|
||
|
||
def get_lambda_source(obj): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used to supports default_factory=lambda: torch.ones(1,1,1,1,1)
etc in rewrite_dataclass_init_default_factories
@@ -1,9 +1,11 @@ | |||
extend = "../pyproject.toml" | |||
exclude = ["notebooks/*.ipynb"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ruff and jupytext sometimes disagree on formatting.
but our ipynbs are autogeneratred anyways.
|
||
dataset = '14173489' | ||
|
||
tmp = tempfile.TemporaryDirectory() # RAII, automatically cleaned up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the directory including any files in it will be deleted when tmp goes out of scope (also on interpreter shutdown.)
downside is that this will print a warning on shutdown to the console
import tempfile | ||
from pathlib import Path | ||
|
||
import zenodo_get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always use zenodo get now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this means we download a lot of data we don't need....
t1_start = rearrange(t1_dictionary[idx_best_match], '(y x)->1 1 y x', y=n_y, x=n_x) | ||
import einops | ||
|
||
dot_product = einops.einsum( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit cleaner dictionary matching
src/mrpro/algorithms/csm/walsh.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some description to the algorithms that describes what they do
Some comments, still have to go through the docu
|
The notebook has changed anyways. The results do not match the challenge submission anymore. I would suggest to link the award mentions to the specific version of the notebooks in git used to create the results. And remove the T2* example.
I want to remove iterative SENSE from "basic". We still use the CSMs from the reference scan with the manual or direct reconstruction, right? |
Co-authored-by: Christoph Kolbitsch <[email protected]>
No, everywhere else we use self-calibrated SENSE |
You can easily launch notebooks via the |colab-badge| badge and give the notebooks a try without having to | ||
install anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can easily launch notebooks via the |colab-badge| badge and give the notebooks a try without having to | |
install anything. | |
You can launch the notebooks using the |colab-badge| badge and explore them directly in browser — no installation required. |
Missing dimensions/shape formatting: mrpro/src/mrpro/data/KNoise.py Line 21 in e029d5e
Line 17 in e029d5e
Line 379 in e029d5e
Line 478 in e029d5e
Line 650 in e029d5e
|
.. toctree:: | ||
:maxdepth: 1 | ||
:maxdepth: 2 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to change the order to
user_guide
examples
contributor_guide
api
faq
\hat{m}_t &= \frac{m_t}{1 - \beta_1^t}, \quad \hat{v}_t = \frac{v_t}{1 - \beta_2^t} \\ | ||
\theta_{t+1} &= \theta_t - \frac{\eta}{\sqrt{\hat{v}_t} + \epsilon} \hat{m}_t | ||
|
||
where: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This list is not displayed correctly
:math:`H:= diag(H_1, ..., H_N)` and :math:`b:= [b_1, ..., b_N]^T`. | ||
1. Initialize the residual :math:`r_0 = b - Hx_0` (with :math:`x_0` as the initial guess). | ||
2. Set the search direction :math:`p_0 = r_0`. | ||
3. For each iteration :math:`k = 0, 1, 2, ...`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List not working
__all__ = ["csm", "optimizers", "prewhiten_kspace", "reconstruction"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think dcf is missing here
.. [Hestenes1952] Hestenes, M. R., & Stiefel, E. (1952). Methods of conjugate gradients for solving linear systems. | ||
*Journal of Research of the National Bureau of Standards*, 49(6), 409-436 | ||
.. [Nocedal2006] Nocedal, J. (2006). *Numerical Optimization* (2nd ed.). Springer. | ||
.. [WikipediaCG] Wikipedia: Conjugate Gradient`<https://en.wikipedia.org/wiki/Conjugate_gradient>_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wiki link not displayed correctly
---------- | ||
.. [NOC1980] Nocedal, J. (1980). "Updating quasi-Newton matrices with limited storage." | ||
*Mathematics of Computation*, 35(151), 773-782. | ||
`10.1090/S0025-5718-1980-0572855-7`<https://doi.org/10.1090/S0025-5718-1980-0572855-7>_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Links not displayed correctly here and below
Example for 2D-Cartesian Trajectories: | ||
kx changes along k0 and is Frequency Encoding | ||
ky changes along k2 and is Phase Encoding | ||
kz is zero(1,1,1,1) | ||
kz is zero `(1,1,1,1)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO does not look nice in the docu
@@ -18,22 +18,22 @@ class KTrajectoryRawShape(MoveDataMixin): | |||
"""K-space trajectory shaped ((other*k2*k1),k0). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shape formating
# Finally, we will reconstruct a Cartesian scan with regular undersampling. | ||
|
||
|
||
# %% tags=["hide-cell"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we want to hide this cell?
# In general, both the data and the reconstruction module must be moved to the same device. | ||
|
||
# %% | ||
if torch.cuda.is_available(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this leads to a fail on colab:
RuntimeError: Expected all tensors to be on the same device, but found at least two devices, cuda:0 and cpu!
and fulfill f(a*x + b*y) = a*f(x) + b*f(y) | ||
with a,b scalars and x,y tensors. | ||
LinearOperators have exactly one input tensors and one output tensor, | ||
and fulfill :math:`f(a*x + b*y) = a*f(x) + b*f(y)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b and y not displayed correctly
Formatting missing:
Can you please also unify the use of l1 norm, L1 norm and L1-norm here? Similar issue here:
|
(… other, coils, z, y, x) and (time … other, coils, z, y, x) need to be correctly formatted for all signal models |
Remove double quotes here?
|
Formatting missing here:
|
@@ -98,8 +90,8 @@ Please check how your new additions render in the documentation before requestin | |||
|
|||
Adding new Examples | |||
=================== | |||
New exciting applications of MRpro can be added in ```examples``` as only ```.py``` files with code-cells. These can, for example, be used in VSCode with the python extension, or in JupyterLab with the `jupytext <https://jupytext.readthedocs.io/en/latest/install.html>`_ extension. | |||
An automatic workflow at github will create notebooks and pages in the documentation based on the python scripts. | |||
New exciting applications of MRpro can be added in ```examples``` as only ```.py``` files with code-cells. These can, for example, be used in VSCode with the python extension, or in JupyterLab with the `jupytext <https://jupytext.readthedocs.io/en/latest/>`_ extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong code formatting
New exciting applications of MRpro can be added in ```examples``` as only ```.py``` files with code-cells. These can, for example, be used in VSCode with the python extension, or in JupyterLab with the `jupytext <https://jupytext.readthedocs.io/en/latest/>`_ extension. | |
New exciting applications of MRpro can be added in ``examples`` as only ``.py`` files with code-cells. These can, for example, be used in VSCode with the python extension, or in JupyterLab with the `jupytext <https://jupytext.readthedocs.io/en/latest/>`_ extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same on the line 87:
You can build the documentation locally via running ```make html``` in the docs folder. The documentation will also be build in each PR and can be viewed online.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And line 100:
Release Strategy
================
We are still in pre-release mode and do not guarantee a stable API / strict semantic versioning compatibility. We currently use ```0.YYMMDD``` as versioning and release in regular intervals to `pypi <https://pypi.org/project/mrpro/>`_.
A bunch of fixes and improvements to the documentation.