search

FiniteVolumeTransportPhenomena / PyFVTool

Finite volume toolbox in Python
GNU Lesser General Public License v2.1
13 stars 4 forks source link

Minimal version number for scipy #13

Closed mhvwerts closed 1 year ago

mhvwerts commented 1 year ago

Unexpectedly, my conda package manager somehow downgraded the installed scipy package in my environment. I am not entirely sure why, but that is a different story.

As a result of this accidental downgrade, I discovered that PyFVTool needs at least scipy >= 1.8.0, since it usesscipy.sparse.csr_array which only became available in version 1.8.0.

After upgrading (un-downgrading...?), I am now on scipy 1.11.2 and PyFVTool happily works again.

So better add this minimal version number to the requirements. I do not readily see where these are defined in the repo.

P.S. This happened when briefly exploring [pytest_notebook]. It was actually this package that unveiled the scipy version bug, so this tool seems to work for testing PyFVTool notebooks!

mhvwerts commented 1 year ago

From version 1.8.0 onwards, SciPy has made significant improvements to the sparse array implementation, it seems. Another good reason to define a minimal version number for scipy.

simulkade commented 1 year ago

That is a very good point. I'll be sure to take care of it in a moment. When I started the development, I had no idea which sparse package to use. I think I read an article about the new improvements in csar_array and how it will be more consistent with numpy arrays so I just used it. I also saw a nice trick by @gmweir that could make pyfvtool work with the older scipy versions.

mhvwerts commented 1 year ago

Thanks for taking care of this (a03e999).

It is a very good thing that pyfvtool only depends on the standard scientific packages numpy, scipy and matplotlib. As such it will readily work with any reasonably up-to-date scientific Python installation. For me, this is important: it makes it easy to share with students and colleagues without running into dependency problems and elaborate installation recipes.

We have the luxury with PyFVTool of being able to 'simply' use a direct method for solving the sparse matrix equation using scipy.sparse.linalg.spsolve (I believe it uses the UMFPACK code which is also used by Matlab -- or it uses SuperLU?). For more demanding systems, there is a choice of iterative solvers already in scipy.sparse (although I wouldn't know how to properly use those...). Farther beyond, I think we would be in FiPy territory and switch to unstructured meshes and high-performance solvers such as PETSc. Probably out of the intended scope of PyFVTool. Your choice to use standard spsolve is an excellent choice for pyfvtool.

A few years ago, scipy.sparse was not in optimal shape. I understand that it is now rapidly improving and even considered a priority (https://scientific-python.org/grants/sparse_arrays/). Better use recent versions of scipy.