Examples typos

[lrlunin]

Fix issues caused by the switch to SI units. Use new qmri challenge dataset Use latex in notebooks

Closes #459 Closes #462

Hello,

there are some very minor changes, but you have to start somewhere :-)

1. The first one is using T1 instead of subscript as done before. Only this example has used it.
2. Use of \ln and \cos TeX commands instead of plain text.

They are not urgent at all. Thanks in advance.

[github-actions[bot]]

Coverage Report

File	Stmts	Miss	Cover	Missing
src/mrpro/algorithms/csm
inati.py	24	1	96%	44
walsh.py	16	1	94%	34
src/mrpro/algorithms/dcf
dcf_voronoi.py	53	4	92%	15, 48–49, 76
src/mrpro/algorithms/optimizers
adam.py	20	1	95%	69
src/mrpro/algorithms/reconstruction
DirectReconstruction.py	28	16	43%	51–71, 85
IterativeSENSEReconstruction.py	13	1	92%	76
Reconstruction.py	50	22	56%	42, 54–56, 80–87, 104–113
RegularizedIterativeSENSEReconstruction.py	41	17	59%	96–100, 114–139
src/mrpro/data
AcqInfo.py	128	3	98%	26, 169, 207
CsmData.py	29	3	90%	15, 82–84
DcfData.py	45	8	82%	18, 66, 78–83
IData.py	67	9	87%	119, 125, 129, 159–167
IHeader.py	75	7	91%	75, 109, 127–131
KHeader.py	153	17	89%	25, 119–123, 150, 199, 210, 217–218, 221, 228, 260–271
KNoise.py	31	15	52%	39–52, 56–61
KTrajectory.py	69	5	93%	178–182
MoveDataMixin.py	137	18	87%	15, 113, 129, 143–145, 207, 305–307, 320, 399, 419–420, 422, 437–438, 440
QData.py	39	7	82%	42, 65–73
Rotation.py	674	35	95%	100, 198, 335, 433, 477, 495, 581, 583, 592, 626, 628, 691, 768, 773, 776, 791, 808, 813, 889, 1077, 1082, 1085, 1109, 1113, 1240, 1242, 1250–1251, 1315, 1397, 1690, 1846, 1881, 1885, 1996
SpatialDimension.py	230	21	91%	33, 103, 128, 135, 141, 261–263, 276–278, 312, 330, 343, 356, 369, 382, 391–392, 407, 416
acq_filters.py	12	1	92%	47
src/mrpro/data/_kdata
KData.py	134	18	87%	109–110, 125, 132, 142, 150, 204–205, 243, 248–249, 268–279
KDataRemoveOsMixin.py	29	2	93%	44, 46
KDataSelectMixin.py	19	2	89%	48, 63
KDataSplitMixin.py	48	3	94%	53, 84, 93
src/mrpro/data/traj_calculators
KTrajectoryCalculator.py	25	2	92%	23, 45
KTrajectoryIsmrmrd.py	13	2	85%	41, 50
KTrajectoryPulseq.py	29	1	97%	54
src/mrpro/operators
CartesianSamplingOp.py	89	3	97%	118, 157, 280
ConstraintsOp.py	60	2	97%	46, 48
EndomorphOperator.py	65	2	97%	228, 234
FiniteDifferenceOp.py	27	2	93%	40, 105
FourierOp.py	157	3	98%	263, 381, 386
Functional.py	71	5	93%	20–22, 117, 119
GridSamplingOp.py	136	9	93%	72–73, 82–83, 90–91, 94, 96, 98
LinearOperator.py	171	12	93%	55, 91, 190, 220, 261, 270, 278, 287, 295, 320, 418, 423
LinearOperatorMatrix.py	158	16	90%	82, 119, 152, 161, 166, 175–178, 191–194, 203, 215, 304, 331, 359
MultiIdentityOp.py	13	2	85%	43, 48
Operator.py	78	2	97%	25, 74
ProximableFunctionalSeparableSum.py	39	3	92%	50, 103, 110
SliceProjectionOp.py	173	8	95%	44, 61, 63, 69, 206, 227, 260, 300
WaveletOp.py	120	5	96%	152, 170, 205, 210, 233
ZeroPadOp.py	16	1	94%	30
src/mrpro/utils
filters.py	62	2	97%	44, 49
slice_profiles.py	46	6	87%	20, 36, 113–116, 149
sliding_window.py	34	1	97%	34
split_idx.py	10	2	80%	43, 47
summarize_tensorvalues.py	11	9	18%	20–29
typing.py	18	11	39%	8–23
zero_pad_or_crop.py	31	6	81%	26, 30, 54, 57, 60, 63
TOTAL	4857	354	93%

Tests	Skipped	Failures	Errors	Time
2250	0 :zzz:	0 :x:	0 :fire:	2m 4s :stopwatch:

[github-actions[bot]]

:books: Documentation

:file_folder: Download as zip :mag: View online

[fzimmermann89]

good idea to make the use of T_1 consistent in the examples -- but in all equations it should be $T_1$.

[lrlunin]

good idea to make the use of T_1 consistent in the examples -- but in all equations it should be $T_1$.

Okay, I'll replace the notation in all examples then.

[lrlunin]

I also added subscript notations for the graphs. Should be ready to merge so far.

[ckolbPTB]

maybe not change the title of the notebooks because these titles are used in the docu as doctree entries which does not support latex formatting.

[lrlunin]

maybe not change the title of the notebooks because these titles are used in the docu as doctree entries which does not support latex formatting.

They actually kinda do, but not on the top level. I guess there is some option to force the TeX display mode. In the above level:

If you check on of the examples.

So I believe there is some page-specific display-/parse-mode which was automatically enabled in the page with the markdown notebook and was not enabled in the examples' table of content. I'll check if there is some magic hint to force the page be displayed as markdown.

[lrlunin]

I found a much better solution. With the nbsphinx all works as expected and all converting, running, including as html in the docs.yml can removed. I'll make a commit till monday I guess.

[ckolbPTB]

I found a much better solution. With the nbsphinx all works as expected and all converting, running, including as html in the docs.yml can removed. I'll make a commit till monday I guess.

They way it is know allows for all the notebooks to be run in parallel. Maybe nbspinx offers this now too?

[lrlunin]

I found a much better solution. With the nbsphinx all works as expected and all converting, running, including as html in the docs.yml can removed. I'll make a commit till monday I guess.

They way it is know allows for all the notebooks to be run in parallel. Maybe nbspinx offers this now too?

They promise to allow threads with -j parameter analog to make parameter.

[fzimmermann89]

Btw: We will still need a conversion from py to ipynb & commit, as we want to keep the notebooks available in the repo as well.

[lrlunin]

I found a much better solution. With the nbsphinx all works as expected and all converting, running, including as html in the docs.yml can removed. I'll make a commit till monday I guess.

They way it is know allows for all the notebooks to be run in parallel. Maybe nbspinx offers this now too?

They promise to allow threads with -j parameter analog to make parameter.

This seems not to be true. The execution of the notebooks is indeed not parallel and lasts approx. 3 minutes on my pc. Then it would be more reasanoble to perform the step with the .py -> .ipynb conversion and further .ipynb evaluation in the action. Then copy and include the evaluated notebooks into the docs with the nbsphinx_execute = 'never' setting in the conf.py.

[fzimmermann89]

My 2cents: Sphinx -j option is not the same as parallel jobs. Both download speed for the example data and CPU should be exhausted by a single example. The parallel jobs run afaik on separate worker machines.

On your local machine at home, the total time might be dominated by the data download..

[lrlunin]

This is done so far and awaits https://github.com/PTB-MR/mrpro/pull/469 to pass the github action since pandoc is missing

[fzimmermann89]

For me, the requirement of a non-pip binary is a blocker here. Does the switch to nbshinx really have such advantages that it is worth the hassle?

Can you maybe first fix the issues with the notebooks you noticed (and maybe also change the code to s instead of ms, this would fix and there open issue #462).

And then we can push the overhaul of the action a bit into the future and talk about it and the advantages your approach has first.

[fzimmermann89]

Try pypandoc-binary

[fzimmermann89]

Do I understand this correctly that remote builds still use papermill to execute (potentially giving us a way to include non public data), but local builds also work and execute the notebooks using nbshinx?

I would add the pandoc binaries as a requirement instead of apt to the docker. this way building just works after a pip install locally.

[fzimmermann89]

Still, consider splitting this in two parts:

• Fixing the notebooks (the fitting and formattin
  • Switching to nbshinx

[fzimmermann89]

consider adding yourself to the pyproject.toml author list

[ckolbPTB]

The T1 mapping using radial golden angle data does not lead to good results anymore.

[ckolbPTB]

The other two quantitative notebooks also don't work anymore..

-> wait for #535 to fix this

[lrlunin]

add yourself to the author list in pyproject.toml

I don't consider the changes done to worth it. They are primary cosmetic.

minor nitpick: in the plots, add a unit to the colorbars for T1 and T2*

TBD

[fzimmermann89]

Of course your decison. I'd say it would be justified. You can also do it in any future pr if you want to.

[fzimmermann89]

@lrlunin: Small hint for next time: Please use imperative mood and a bit more descriptive name for the commit ;)