vidarsko / ComFiT

A python library for simulating field theories with topological defects
MIT License
3 stars 3 forks source link

Error in `base_system_make_your_own_model.ipynb` #13

Closed flokno closed 1 month ago

flokno commented 1 month ago

Hi,

I am running the base_system_make_your_own_model tutorial on my own laptop and get an error when trying to create the gif in the cell

# Initiating a system with positive r value
ls = LandauSystem(2, 0.5)

# Setting an initial condition with both positive and negative values
ls.psi = np.random.rand(ls.xRes, ls.yRes) - 0.5
ls.psi_f = sp.fft.fftn(ls.psi)

# Make animation
for n in range(100):
    ls.evolve(2)
    plt.clf()  # Clearing current figure before plotting new one
    ls.plot_field(ls.psi)
    cf.tool_save_plot(n)
cf.tool_make_animation_gif(n, name="evolution_positive_r")

The error reads (a bit annoying output from vscode):

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[8], [line 14](vscode-notebook-cell:?execution_count=8&line=14)
     [12](vscode-notebook-cell:?execution_count=8&line=12)     ls.plot_field(ls.psi)
     [13](vscode-notebook-cell:?execution_count=8&line=13)     cf.tool_save_plot(n)
---> [14](vscode-notebook-cell:?execution_count=8&line=14) cf.tool_make_animation_gif(n, name="evolution_positive_r")

File ~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/comfit/tools/tool_animation.py:90, in tool_make_animation_gif(counter, name, fps)
     [88](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/comfit/tools/tool_animation.py:88) for image_file in image_files:  
     [89](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/comfit/tools/tool_animation.py:89)     img.append(imageio.imread(image_file))
---> [90](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/comfit/tools/tool_animation.py:90) imageio.mimsave(name, img, fps=fps, loop=0) 
     [92](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/comfit/tools/tool_animation.py:92) # Delete the png files
     [93](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/comfit/tools/tool_animation.py:93) for file in image_files:

File ~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/v2.py:495, in mimwrite(uri, ims, format, **kwargs)
    [493](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/v2.py:493) imopen_args["legacy_mode"] = True
    [494](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/v2.py:494) with imopen(uri, "wI", **imopen_args) as file:
--> [495](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/v2.py:495)     return file.write(ims, is_batch=True, **kwargs)

File ~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/plugins/pillow.py:417, in PillowPlugin.write(self, ndimage, mode, format, is_batch, **kwargs)
    [414](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/plugins/pillow.py:414)     kwargs["duration"] = 1000 * 1 / kwargs.get("fps")
    [416](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/plugins/pillow.py:416) if isinstance(ndimage, list):
--> [417](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/plugins/pillow.py:417)     ndimage = np.stack(ndimage, axis=0)
    [418](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/plugins/pillow.py:418)     is_batch = True
    [419](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/imageio/plugins/pillow.py:419) else:
...
--> [449](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/numpy/core/shape_base.py:449)     raise ValueError('all input arrays must have the same shape')
    [451](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/numpy/core/shape_base.py:451) result_ndim = arrays[0].ndim + 1
    [452](https://file+.vscode-resource.vscode-cdn.net/Users/flokno/working/projects/peer_review/JOSS/6599_comfit/ComFiT/tutorial/~/working/projects/peer_review/JOSS/6599_comfit/ComFiT/venv/lib/python3.9/site-packages/numpy/core/shape_base.py:452) axis = normalize_axis_index(axis, result_ndim)

ValueError: all input arrays must have the same shape

The error apparently has something to do with pillow, of which I have version 10.3.0 installed.

I attach a list of my library versions for reference

packages.txt

flokno commented 1 month ago

edit: it seems to work in the colab notebook. will check my local setup

flokno commented 1 month ago

It seems that my images have slightly different sizes when saved locally, which then produces the error.

I changed comfit.tools.tool_animation.tool_make_animation to

def tool_make_animation_gif(counter, name=None, fps=24):
    """
    Creates an animation from a series of plot images and saves it as a GIF file.

    Input:
        - counter (int): The number of plot images to include in the animation.
        - name (str, optional): The filename for the output video. Defaults to today's date followed by ' - output_video.mp4'.
        - fps (int, optional): The frames per second for the video. Defaults to 24.
    """

    if name is None:
        name = datetime.now().strftime("%y%m%d_%H%M") + ' - output_animation.gif'
    else:
        name = datetime.now().strftime("%y%m%d_%H%M") + ' - ' + name + '.gif'

    image_files = [f'plot_{counter}.png' for counter in range(counter)]
    images = []
    for image_file in image_files:  
        print(image_file)
        image = imageio.imread(image_file)
        print(image.shape)
        images.append(image)
    imageio.mimsave(name, images, fps=fps, loop=0) 

    # Delete the png files
    for file in image_files:
        os.remove(file)

then I get the output

plot_0.png
(435, 525, 4)
plot_1.png
(435, 525, 4)
plot_2.png
(435, 533, 4)
plot_3.png
(435, 533, 4)
plot_4.png
(435, 542, 4)
plot_5.png
(435, 542, 4)
...

i.e., the size of the images has changed

vidarsko commented 1 month ago

Thank you for raising the issue. It seems that the cf.tool_save_plot function might save figures in different formats depending on a users specific setup. I have changed the tool_save_plot-function to take as optional inputs image_size_inches=(6,5)' anddpi=100' (dots per inch). If not provided explicitly, this should set the resolution of the plots to 600x500 which will hopefully fix the issue you are having. I tried it on my computer setup and it produced a .gif with the correct dimensions.

Can you pull the main branch and run your script again and see if it fixes your issue?

flokno commented 1 month ago

It does not work:

a) the wrong variable name is used in https://github.com/vidarsko/ComFiT/blob/main/comfit/tools/tool_animation.py#L33 (image_size vs. image_size_inches).

b) If I fix that, I still get the same error as before. I suspect that the images get cropped when the colorbar labels get too many digits. I attach the first 5 pngs I get below. You can see that they have different widths when the number of digits increases

plot_1 plot_2 plot_3 plot_4 plot_5

vidarsko commented 1 month ago

It does not work:

a) the wrong variable name is used in https://github.com/vidarsko/ComFiT/blob/main/comfit/tools/tool_animation.py#L33 (image_size vs. image_size_inches).

b) If I fix that, I still get the same error as before. I suspect that the images get cropped when the colorbar labels get too many digits. I attach the first 5 pngs I get below. You can see that they have different widths when the number of digits increases

plot_1

plot_2

plot_3

plot_4

plot_5

Strange! It seems your setup with matplotlb ignores the custom size that I tried to impose. Can you try to vary image_size_inches and dpi and see if you get any changes?

flokno commented 1 month ago

I tried different image_size_inches, no effect.

I could make it work by forcing the colorbar not to push the image boundaries, e.g., like this

from matplotlib.ticker import FuncFormatter

...

            if colorbar:
                _fmt = lambda x, pos: '{:15f}'.format(x)
                cbar = plt.colorbar(pcm, ax=ax, format=FuncFormatter(_fmt), fraction=0.046, pad=0.04)

Now, I would recommend to find a more robust solution to create the GIFs than relying on matplotlib to create images of exact same pixel count. This seems to be an open issue in matplotlib, see e.g. https://github.com/matplotlib/matplotlib/issues/8543

vidarsko commented 1 month ago

I have added another check in the tool_make_animation_gif, whereby it checks if all the images are of the same dimensions and crops them if not. As a side note, I realized that since imageio relies on the pillow package, which can do the same job, I was able to remove the dependency on imageio from comfit, which should be a little bit more stable.

I was not able to reproduce the error with the packages you listed, but let me suggest three ways of exploring this issue:

1) Can you try to run comfit with this latest patch and see if the proposed patch works (even though it is not super elegant)?

2) I see that in the list of dependencies, you have comfit==1.4.2 installed. Is it possible that when you are running the script with import comfit, that uses the current version from pypi (which does not have the patch implemented yet)? Could you make a clean virtual environnement with a clone of the github repository and only the required packages and see if it fixes the issue?

I agree that saving images as pngs and then to export them is not an optimal strategy, and I will think of future ways of making it better. If neither step 1 or 2 works, would you be open to leaving this issue open and still continuing with the submission of the paper, or do you think that this is essential for the package to be published in JOSS?

flokno commented 1 month ago

I have added another check in the tool_make_animation_gif, whereby it checks if all the images are of the same dimensions and crops them if not. As a side note, I realized that since imageio relies on the pillow package, which can do the same job, I was able to remove the dependency on imageio from comfit, which should be a little bit more stable.

I was not able to reproduce the error with the packages you listed, but let me suggest three ways of exploring this issue:

1. Can you try to run comfit with this latest patch and see if the proposed patch works (even though it is not super elegant)?

I think it works now. I see the warning when the files are cropped, and I get the GIFs 👍

2. I see that in the list of dependencies, you have comfit==1.4.2 installed. Is it possible that when you are running the script with `import comfit`, that uses the current version from pypi (which does not have the patch implemented yet)? Could you make a clean virtual environnement with a clone of the github repository and only the required packages and see if it fixes the issue?

I was working with a clean venv and commented out the pip install.

I agree that saving images as pngs and then to export them is not an optimal strategy, and I will think of future ways of making it better. If neither step 1 or 2 works, would you be open to leaving this issue open and still continuing with the submission of the paper, or do you think that this is essential for the package to be published in JOSS?

I think making the GIFs is a very nice feature so it is nice when it actually works. This seems to be the case now!