-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incorrect score using Mac with ARM CPU #67
Comments
Also, not sure if related but, after calculating the score,
|
@romainsacchi This is unrelated, instead of |
Here is a minimal test case to reproduce from 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:
On ARM:
The second zero is incorrect, it should be the max signed 32 bit integer (4294967295). |
A simpler test: np.array([-1], dtype=np.float32).astype(np.uint32) Gives |
Right, so that's a |
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. |
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. |
More context for Tomás's comment: All numpy types which overlap standard Python types ( |
Using a Mac computer with a M1 chip, and the following modules:
the following:
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')
.The text was updated successfully, but these errors were encountered: