Re-write broken plot_geometry.py with many enhancements, and bug fixes

yliu88au commented 2 years ago

Rewrite broken plot_geometry.py with numerous enhancements: limited angles, detector rotation and offsets, origin offsets, animation (and save movie to a file), tested by test_plot_geometry.py with various geometries (#157, #191). Please check implementation of geo.COR is correct.
Add circular tomosynthesis and linear tomosynthesis geometries in common_geometry.py #280
Merged FISTA and FISTA_mod into one for MATLAB #211
Fixed many small bugs in both MATLAB and Python versions (#335, #272, and new bugs)

yliu88au commented 2 years ago

To save animation file, one needs to install ffmpeg in python

AnderBiguri commented 2 years ago

This is excellent work as usual. I am now temporarily unable to test the python version of Tigre, give me few days to get around this and I'll test it and merge it.

yliu88au commented 2 years ago

Here are some animations of different geometries:

Tomosynthesis: https://user-images.githubusercontent.com/75292881/156271564-dfed478b-01b3-4f3c-bd8c-ae194171be57.mp4

Linear Tomosynthesis: https://user-images.githubusercontent.com/75292881/156271571-410314ad-4159-490c-80da-9c8165010876.mp4

Helical_CT: https://user-images.githubusercontent.com/75292881/156271577-0848093c-9561-47a0-888c-c265a1cb2ae5.mp4

AnderBiguri commented 2 years ago

When trying to run demo 1 in windows with python 3.7, I see nothing. The code runs successfully but I see nothing visualized.

When trying to run test_plot_geometry.py I get a series of indexing errors, but the start of the message is:

MovieWriter ffmpeg unavailable; using Pillow instead.

However, I have installed ffmpeg via pip. Any idea?

yliu88au commented 2 years ago

My test environment is Anaconda Python 3.9, using Spyder to run the test_plot_geometry.py, and with Ipython graphic setting to Automatic (floating graphs, not embedded). I installed the ffmpeg by "conda install ffmpeg"

AnderBiguri commented 2 years ago

OK, this may be an IDE thing.

I want to keep TIGRE as "vanilla" as possible, so lets try to see if we can change the code such that it runs with standard python (not anaconda) and when just being called via command line.

yliu88au commented 2 years ago

Even under Spyder, you need to assign a variable for output of ani=plot_geometry(), then you have to run the returned object "ani" to see the animation. This may be the reason you don't see anything from Demo1?

AnderBiguri commented 2 years ago

I have not changed your code, just ran the first demo and the test script, and I believe both of those do what you say. I assume they should work as is, right?

yliu88au commented 2 years ago

I got demo1 working from command windows, just need to import "from matplotlib import pyplot as plt" at beginning and add the following at end of script: "plt.show()"

AnderBiguri commented 2 years ago

That gets me somewhere, but I don't get the animations running. So for the demo 1, I get two static images, and the following error:

Traceback (most recent call last):
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\tkinter\__init__.py", line 1705, in __call__
    return self.func(*args)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\tkinter\__init__.py", line 749, in callit
    func(*args)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\backends\_backend_tk.py", line 253, in idle_draw
    self.draw()
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 9, in draw
    super(FigureCanvasTkAgg, self).draw()
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\backends\backend_agg.py", line 407, in draw
    self.figure.draw(self.renderer)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\artist.py", line 41, in draw_wrapper
    return draw(artist, renderer, *args, **kwargs)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\figure.py", line 1870, in draw
    self.canvas.draw_event(renderer)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\backend_bases.py", line 1759, in draw_event
    self.callbacks.process(s, event)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\cbook\__init__.py", line 229, in process
    self.exception_handler(exc)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\cbook\__init__.py", line 81, in _exception_printer       
    raise exc
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\cbook\__init__.py", line 224, in process
    func(*args, **kwargs)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 975, in _start
    self._init_draw()
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 1719, in _init_draw
    self._draw_frame(next(self.new_frame_seq()))
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 1742, in _draw_frame
    self._drawn_artists = self._func(framedata, *self._args)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\pytigre-2.2.0-py3.7-win-amd64.egg\tigre\utilities\visualization\plot_geometry.py", line 135, in update
    stext.set_position_3d(stj[pos,:]+15)
AttributeError: 'Text3D' object has no attribute 'set_position_3d'

yliu88au commented 2 years ago

This might be missing some of the plot library, I think you may need tk/QT5 backend installed. Anaconda come with it. My setup includes "tk 8.6.12" and "qt 5.12.9"

AnderBiguri commented 2 years ago

Right, that could be it, I will investigate. However, I don't want to have anaconda as a requirement for TIGRE, and even less tk/QT5. As is, TIGRE can run on clusters and when you ask it to plot, it will save into disk if there is not graphic interface. This is very useful for many users, so I'd like to keep that.

Worse case scenario (i.e. if we cant make your code work as vanilla as I wished), we need to at least make sure it does not error, but catch it and have warnings that XX machine does not support animations etc.

yliu88au commented 2 years ago

Yes, you can save it as *.mp4 file, there is an option in plot_gemoetry(..., fname='yourana file'). Default is None, which does not save file. Need to check

AnderBiguri commented 2 years ago

Yes, but that should happen automatically if the alternative is an error. Like in

https://github.com/CERN/TIGRE/blob/57feb16f75dc8f00ff25c8c0ff010e374c9a0652/Python/tigre/utilities/visualization/plotimg.py#L43-L69

yliu88au commented 2 years ago

Could you please test d01 on your system by replacing the last line 'plt.show()' with ani.save("d01_CreateGeometry.mp4")

On my system, it does not plot, rather it save the animation file.

AnderBiguri commented 2 years ago

The backend still seems to screw me here:

MovieWriter ffmpeg unavailable; using Pillow instead.
Traceback (most recent call last):
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 251, in saving
    yield self
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 1144, in save
    anim._init_draw()  # Clear the initial frame
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 1719, in _init_draw
    self._draw_frame(next(self.new_frame_seq()))
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 1742, in _draw_frame
    self._drawn_artists = self._func(framedata, *self._args)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\pytigre-2.2.0-py3.7-win-amd64.egg\tigre\utilities\visualization\plot_geometry.py", line 135, in update
    stext.set_position_3d(stj[pos,:]+15)
AttributeError: 'Text3D' object has no attribute 'set_position_3d'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".\demos\d01_CreateGeometry.py", line 140, in <module>
    ani.save("d01_CreateGeometry.mp4")
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 1161, in save
    writer.grab_frame(**savefig_kwargs)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\contextlib.py", line 130, in __exit__
    self.gen.throw(type, value, traceback)
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 253, in saving
    self.finish()
  File "C:\Users\Ander\AppData\Local\Programs\Python\Python37\lib\site-packages\matplotlib\animation.py", line 554, in finish
    self._frames[0].save(
IndexError: list index out of range

yliu88au commented 2 years ago

Could it be the feature 'Text3D' causing the problem? we don't have to use letters to mark Source, Detector, and Origin, I guess.... (nice to have though).

AnderBiguri commented 2 years ago

It could be! unfortunately I don't have much time this week to dig into it to fix it. If you find time, great, otherwise I'll need to see if I can do so a bit later. I only have time to do basic tests to see if it works.

yliu88au commented 2 years ago

I have replaced 'set_position_3d()' with 'set_position()' in text update. It works for my system setup, could you test it on yours?

AnderBiguri commented 2 years ago

I still get MovieWriter ffmpeg unavailable; using Pillow instead. and then errors. Maybe this is a thing in my system, I have installed the package, but still getting the error.... Or maybe it only works with conda.

yliu88au commented 2 years ago

Conda only installed ffmpeg ver 4.3.1. Could you check your version of ffmpeg and perhaps install this version to test? There are newer versions available on the site ffmpeg.org, but not sure what is the latest "python" version. It seems pip will install version 1.4, may be this is too old.

AnderBiguri commented 2 years ago

OK, this made some progress. If I change the exception type to IndexError then I can run test_plot_geometry.py, but it finds no engine, even if I have them installed. Also it shows nothing at all.

However, demo 1 still crashes at the ani.save() line....

AnderBiguri commented 2 years ago

Sorry @yliu88au I have been extremely busy lately. I'd love to merge this, but the plotting side still has some work to do to be compatible with HPCs and other python distributions.

A good alternative would be (if you are willing to do it) is to break this PR into 2: one for the plotting, another for the rest of the things. I can easily merge the second, and leave the first for when I have a bit more time. Do let me know if this seems like a thing you can do. Thanks a lot again for the amazing work so far! I genuinely appreciate it.

yliu88au commented 2 years ago

I will look into splitting

CERN / TIGRE

Re-write broken plot_geometry.py with many enhancements, and bug fixes #357