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

Incorrect score using Mac with ARM CPU #67

Open romainsacchi opened 1 year ago

romainsacchi commented 1 year ago

Using a Mac computer with a M1 chip, and the following modules:

brightway2                2.4.2                    pypi_0    pypi
bw-migrations             0.2                      pypi_0    pypi
bw2analyzer               0.10                     pypi_0    pypi
bw2calc                   1.8.1                    pypi_0    pypi
bw2data                   3.6.5                    pypi_0    pypi
bw2io                     0.8.6                    pypi_0    pypi

the following:


db = bw.Database("ecoinvent 3.8 cutoff")
a = [a for a in db if "dehydrogenation of butan" in a["name"] and a["reference product"] == "butyrolactone"][0]
lca = bw.LCA({a :1}, ('ReCiPe Midpoint (H) V1.13', 'human toxicity', 'HTPinf'))
lca.lci()
lca.lcia()
lca.score

returns an incorrect score (>465'000 instead of ~1.8).

Almost 100% of the score is due to '1,4-Butanediol' (kilogram, None, ('air', 'urban air close to ground')) which does not have a CF in ('ReCiPe Midpoint (H) V1.13', 'human toxicity', 'HTPinf').

romainsacchi commented 1 year ago

Also, not sure if related but, after calculating the score, .top_emissions() fails:


---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/scipy/sparse/_sputils.py:221, in isintlike(x)
    220 try:
--> 221     operator.index(x)
    222 except (TypeError, ValueError):

TypeError: 'numpy.float64' object cannot be interpreted as an integer

During handling of the above exception, another exception occurred:

ValueError                                Traceback (most recent call last)
Cell In[20], line 1
----> 1 lca.top_emissions()

File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/bw2calc/lca.py:575, in LCA.top_emissions(self, **kwargs)
    573 except ImportError:
    574     raise ImportError("`bw2analyzer` is not installed")
--> 575 return ContributionAnalysis().annotated_top_emissions(self, **kwargs)

File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/bw2analyzer/contribution.py:152, in ContributionAnalysis.annotated_top_emissions(self, lca, names, **kwargs)
    146 """Get list of most damaging biosphere flows in an LCA, sorted by ``abs(direct impact)``.
    147 
    148 Returns a list of tuples: ``(lca score, inventory amount, activity)``. If ``names`` is False, they returns the process key as the last element.
    149 
    150 """
    151 ra, rp, rb = lca.reverse_dict()
--> 152 results = [
    153     (score, lca.inventory[index, :].sum(), rb[index])
    154     for score, index in self.top_emissions(
    155         lca.characterized_inventory, **kwargs
    156     )
    157 ]
    158 if names:
    159     results = [(x[0], x[1], get_activity(x[2])) for x in results]

File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/bw2analyzer/contribution.py:153, in <listcomp>(.0)
    146 """Get list of most damaging biosphere flows in an LCA, sorted by ``abs(direct impact)``.
    147 
    148 Returns a list of tuples: ``(lca score, inventory amount, activity)``. If ``names`` is False, they returns the process key as the last element.
    149 
    150 """
    151 ra, rp, rb = lca.reverse_dict()
    152 results = [
--> 153     (score, lca.inventory[index, :].sum(), rb[index])
    154     for score, index in self.top_emissions(
    155         lca.characterized_inventory, **kwargs
    156     )
    157 ]
    158 if names:
    159     results = [(x[0], x[1], get_activity(x[2])) for x in results]

File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/scipy/sparse/_index.py:47, in IndexMixin.__getitem__(self, key)
     46 def __getitem__(self, key):
---> 47     row, col = self._validate_indices(key)
     49     # Dispatch to specialized methods.
     50     if isinstance(row, INT_TYPES):

File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/scipy/sparse/_index.py:152, in IndexMixin._validate_indices(self, key)
    149 M, N = self.shape
    150 row, col = _unpack_index(key)
--> 152 if isintlike(row):
    153     row = int(row)
    154     if row < -M or row >= M:

File /opt/homebrew/Caskroom/miniforge/base/envs/commute/lib/python3.10/site-packages/scipy/sparse/_sputils.py:229, in isintlike(x)
    227     if loose_int:
    228         msg = "Inexact indices into sparse matrices are not allowed"
--> 229         raise ValueError(msg)
    230     return loose_int
    231 return True

ValueError: Inexact indices into sparse matrices are not allowed
cmutel commented 1 year ago

@romainsacchi This is unrelated, instead of (score, lca.inventory[index, :].sum(), rb[index]) it wants (score, lca.inventory[int(index), :].sum(), rb[index]).

cmutel commented 1 year ago

Here is a minimal test case to reproduce from index_with_arrays. This is reproducible on Brightway 2, but Brightway 2.5 uses something very similar. Setup:

import numpy as np
MAX_INT_32 = 4294967295

mapping = {10: 0, 11: 11, 12: 12, 13: 15}

keys = np.array(list(mapping.keys()))
values = np.array(list(mapping.values()))

index_array = np.zeros(keys.max() + 1) - 1
index_array[keys] = values

Functional code:

array_from = np.array([11, 10, 2, 13], dtype=np.uint32)
array_to = np.array([MAX_INT_32, MAX_INT_32, MAX_INT_32, MAX_INT_32], dtype=np.uint32)

mask = array_from <= keys.max()
array_to[:] = -1
array_to[mask] = index_array[array_from[mask]]
array_to[array_to == -1] = MAX_INT_32

array_to

On x64, this gives:

array([ 11, 0, 4294967295, 15], dtype=uint32)

On ARM:

array([11, 0, 0, 15], dtype=uint32)

The second zero is incorrect, it should be the max signed 32 bit integer (4294967295).

cmutel commented 1 year ago

A simpler test:

np.array([-1], dtype=np.float32).astype(np.uint32)

Gives 4294967295 on x64, 0 on ARM.

romainsacchi commented 1 year ago

Right, so that's a numpy issue...

cmutel commented 1 year ago

@tngTUDOR Wow, our first Numpy bug. A big step!

In any case we need to fix the code on our end, we were relying on a sketchy behaviour...

cmutel commented 1 year ago

The Numpy bug was closed as we were using undefined behaviour, for which there is no expectation of consistency. We will need to modify the code accordingly.

What Brightway 2.5 does is to use signed integers, so the -1 mask value doesn't overflow but just stays -1; we should adopt this approach. It will require re-processing all databases and methods, but we already have a migration for this.

tngTUDOR commented 1 year ago

Also, bw2data uses np.bool that will sometime soon be retired. It would be nice if we could at the same time take a look at this.

cmutel commented 1 year ago

More context for Tomás's comment: All numpy types which overlap standard Python types (int, float, bool, etc.) are deprecated as of Numpy 1.20.