search

brightway-lca / brightway2-calc

The calculation engine for the Brightway2 life cycle assessment framework.
BSD 3-Clause "New" or "Revised" License
11 stars 14 forks source link

Add tests for `scikit-umfpack` and `sparselu` solvers #84

Closed cmutel closed 7 months ago

cmutel commented 7 months ago

The current tests assume the presence of pypardiso. We should also test with alternate solvers in use by the community.

cmutel commented 7 months ago

The file that needs to be changed is python-test.yml. This uses pip install .[testing] for installation before testing, which in turn installs pypardiso.

I think we need to change the matrix section to have another dimension with the different solver libraries. On x64 machines (i.e. Windows and Linux) we should test with pypardiso, scikit-umfpack, and the default SuperLU solvers. On ARM (i.e. MacOS) pypardiso is not available. However, for now it seems like the MacOS test machines are still x64, as pypardiso can be installed. So for now we should test all three options on all three OSes.

bw2calc will use the "best" avaiable, so SuperLU will only be used if the other two aren't installed.

cmutel commented 7 months ago

Fixed in 8f3d54db3d920836a18e2b1dd558753f20bfe821

raphaeljolivet commented 4 months ago

Hi,

About that, I was having awfull performances when moving from brightway2.3 to brightway2.45. I thought it was a regression or something.

It appears I installed it with "pip" (not conda) and that no helper librairiy is installed by default.

After installing pypardiso by hand, performances were good again.

That's really something only advanced users are aware of, and to be honest I am lost myself between pypardiso / scikit-umfpack / MKL.

I think I would be nice to have :

What do you think ?

cmutel commented 4 months ago

@raphaeljolivet Unfortunately, Brightway can't be easily installed on Apple Silicon right now (you need to install some dependencies manually, see https://docs.brightway.dev/en/latest/content/installation/index.html). So though we can add optional dependencies to pyproject.toml (we don't use setup.py anymore 😄 ), it is easier to just fix the installation documentation to include a high-performance sparse linear algebra library.

You are welcome to add a PR which gives a check to the __init__.py of bw2calc; it should try to import scikits.umfpack to see it that is available, as the current import uses some magic with scipy and you don't see umfpack directly.

cmutel commented 2 months ago

See also https://github.com/brightway-lca/brightway2-calc/commit/dc336e87e2bc50d193386fdbbe3e74b2f347c239 which checks the error message based on system architecture.