SyneRBI / SIRF-Exercises

SIRF Training and demonstration material
http://www.ccpsynerbi.ac.uk
Apache License 2.0
17 stars 21 forks source link

Remaining Jupyter notebooks issues #156

Closed evgueni-ovtchinnikov closed 2 years ago

evgueni-ovtchinnikov commented 2 years ago

ModuleNotFoundError: No module named 'cil.plugins.astra'

- [x] #150
- [ ] Synergistic/Solutions/Dual_PET_Answer-noMotion:

error: ??? "'ProjData::read_from_file: error opening file FDG_sino_noisy.hs' exception caught at line 327 of /home/sirfuser/devel/buildVM/sources/SIRF/src/xSTIR/cSTIR/cstir.cpp; the reconstruction engine output may provide more information"

- [ ] Synergistic/Solutions/Dual_PET_Answer-wMotion:

FileNotFoundError Traceback (most recent call last)

in 1 # Get to correct directory ----> 2 os.chdir(exercises_data_path('Synergistic')) 3 4 # copy files to working folder and change directory to where the output files are 5 shutil.rmtree('working_folder/dual_PET_noMotion',True) FileNotFoundError: [Errno 2] No such file or directory: '/home/sirfuser/devel/SIRF-Exercises/data/Synergistic' ```
KrisThielemans commented 2 years ago

@samdporter can you help with the path issues above?

KrisThielemans commented 2 years ago

actually path problems are probably #148

samdporter commented 2 years ago

Sure, I'll have a look

epapoutsellis commented 2 years ago

I checked all the notebooks for PSMR on a new cloud instance that @paskino gave me. All the notebooks in the Introductory, Geometry, MR, PET, Synergestic folders are ok. However,

1) In almost all of the notebooks there is %matplotlib notebook which atm raises Javascript Error: IPython is not defined without showing the figures. In some notebooks both %matplotlib notebook and %matplotlib inline are present which is very strange. I suggest to remove these commands.

2) In all notebooks, the headers contain information about Apache licence and Authorship. This takes a lot of space and is not very useful for our users during the training. So I suggest to delete Apache licence and keep one cell with authorship.

@samdporter , @KrisThielemans , @paskino , @DANAJK

KrisThielemans commented 2 years ago

Thanks! Please do not delete license info. We could shorten it, but let's not do that now.

KrisThielemans commented 2 years ago

To be honest, I also wouldn't edit matplotlib statements due to git fun with notebooks (committers need to have followed nbstripout instructions)

paskino commented 2 years ago

I think the license should be written in verbatim, so that it's clear what it is.

The %matplotlib notebook problem is due to some software issue with the image I create, and so far didn't manage to fix it.

paskino commented 2 years ago

BTW there are #161 and #162 PRs to the notebooks that we should consider merging before the school.

danieldeidda commented 2 years ago

Hi, slightly unrelated. I can see that the NPL data is in the cloud but while I can access data downloaded from the download_data.sh I cannot do the same for the real data

epapoutsellis commented 2 years ago

Just for future training sessions, having a cell with Apache information is not good. In some notebooks, there is an Apache cell and then followed by important information about the notebook and is really confusing. For instance in the a_geometry notebook. It's better if we add a license to the repo and remove this cell from every notebook.

paskino commented 2 years ago

We should also add the following in order to reduce the output in the notebooks


from sirf.STIR import MessageRedirector

msg_red = MessageRedirector('info.txt', 'warn.txt', 'errr.txt')
KrisThielemans commented 2 years ago

We should also add the following in order to reduce the output in the notebooks ...

done in 4396a9e89e57ca6098635f5a1f8b2df1e73a4a6b

KrisThielemans commented 2 years ago

Just for future training sessions, having a cell with Apache information is not good. In some notebooks, there is an Apache cell and then followed by important information about the notebook and is really confusing. For instance in the a_geometry notebook. It's better if we add a license to the repo and remove this cell from every notebook.

sorry. License statements should be in each file to the best of my understanding.

KrisThielemans commented 2 years ago

@paskino does %matplotlib widget work? If so, I would NOT modify all notebooks, but create a separate issue "document when to use matplotlib notebook/inline or widget " and add a statement to the HackMD page to use widget

KrisThielemans commented 2 years ago

Introductory/acquisition_model_mr_pet_ct.ipynb astra problem should be resolved.

Synergistic/Solutions/Dual_PET_Answer-noMotion could be path issue, but also depends on previous notebook being run. Ideally we add a statement in the notebook accordingly. @evgueni-ovtchinnikov can you please check the current status, and create a separate issue if it still fails?

KrisThielemans commented 2 years ago

all seems ok, so closing