Skip to content
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

ENH: Use the semantic version instead of the C-API version in the error message for C-API incompatibility #21901

Open
joaopfg opened this issue Jul 1, 2022 · 13 comments

Comments

@joaopfg
Copy link
Contributor

joaopfg commented Jul 1, 2022

Proposed new feature or change:

In the module generate_numpy_api, which is inside the sub package numpy.core.code_generators , there is the generation of the C method static int _import_array(void), which has an if statement like the following:

if (NPY_FEATURE_VERSION > PyArray_GetNDArrayCFeatureVersion()) {
      PyErr_Format(PyExc_RuntimeError, "module compiled against "\
             "API version 0x%%x but this version of numpy is 0x%%x", \
             (int) NPY_FEATURE_VERSION, (int) PyArray_GetNDArrayCFeatureVersion());
      return -1;
  }

I would like to change this error message to, instead of telling about the C-API version, tell about the semantic version of the numpy package. The motivation is that the error seems obscure when using this hexadecimal version that seems to have no relation to the semantic version. Would it be welcome ?

If so, I can try a PR. However, how could I trigger this error for the matter of testing my solution ? Also, is there some way to get the semantic version from the C-API ? Or should I introduce a new macro for it ?

@seberg
Copy link
Member

seberg commented Jul 5, 2022

@joaopfg can you explain how the changed version will be more useful to users? The C-API version is the only relevant version here or am I misunderstanding what you are suggesting to change?

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 5, 2022

@seberg I think giving the C-API version confuses the regular user. See, for example, this kind of doubt on StackOverflow: https://stackoverflow.com/questions/48054531/runtimeerror-module-compiled-against-api-version-0xc-but-this-version-of-numpy

There are many similar to this one. The users get confused when they see this hexadecimal number that seems to have no relation to the semantic version. If we use the semantic version, the user will have a better idea that he needs to upgrade or downgrade the version of his numpy.

@seberg
Copy link
Member

seberg commented Jul 5, 2022

The semantic version is not the interesting one, though. Maybe you can suggest a rephrase that includes the current versions as is?

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 5, 2022

@seberg Please, correct me if I am wrong but, if I understood, the C-API versions set is smaller than the semantic versions set, each semantic version can be mapped to only one C-API version and each C-API version can be mapped to a range of semantic versions.

In this sense, besides telling about both C-API versions, the error message could also tell about the corresponding semantic version ranges of each C-API version so that the user knows to which semantic versions he can upgrade or downgrade his package to get the C-API version he needs. But how to get this mapping from the C-API versions set to the semantic versions set using the C-API ?

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 6, 2022

@seberg The rephrase would be something like RuntimeError: module compiled against C-API version v1 but this C-API version of numpy is v2. Consider upgrade/downgrade your numpy version to some version in the range v3-v4.

@seberg
Copy link
Member

seberg commented Jul 6, 2022

Yeah, maybe giving both versions may make sense. If we touch this, I would prefer pointing to more information. Extension modules should be compiled against the oldest version of NumPy that they can run with, and one way of doing so is oldest-support-numpy

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 6, 2022

@seberg But what if the C-API version needed needs a semantic version in a range [version1, version2] but oldest-supported-numpy is outside of this range ? Should extension modules be compiled against version1 in this case ?

@seberg
Copy link
Member

seberg commented Jul 6, 2022

Not sure I follow. Extension modules must be compiled against the oldest NumPy version they wish to (or can) support. IMO if you see this error you may have:

  1. A bad extension "wheel" (binary install) that should use oldest-support-numpy (with manual constraints if necessary) to build their binary packages.
  2. There is an environment issue messing with package versions, or incompatible package versions were somehow enforced manually
  3. An extension module was compiled locally against a too new version and then NumPy downgraded, or the compiled extension was copied to a different computer with an older NumPy versions manually.

My main point is: The NumPy version seems unlikely to be the main problem, although updating it can be a work-around. (And in the last point it actually may make sense to upgrade NumPy if you want two identical setups.)

(In a sense, if an extension module wants a newer version of NumPy, that module may want to raise a more informative error itself, this error IMO is more about "your extension module was compiled badly" then "your numpy version doesn't fit".)

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 6, 2022

@seberg I see. My point is more like a pragmatic suggestion to the user.

In the case that upgrading the numpy version can be a work around, I think it's worth mentioning it to the user so that he can get a quick fix. Otherwise, the user would have to create an issue, propose a PR or implement a local fix on the package that contains the problematic numpy extension module so that he gets a package with a good compiled extension module. I think most regular users will be afraid of that and will instead go to StackOverflow to get an answer telling them to upgrade the numpy version as a work around =)

Better save the user some time telling them directly in the error message that upgrading the numpy version can be a work around.

@mattip
Copy link
Member

mattip commented Jul 6, 2022

Changing the error message to suggest updating the numpy version may sometimes help, but it may also create new unexpected problems. Rarely does a user only have one package that relies on NumPy, and it is notoriously hard to resolve dependencies between various tensorflow/pytorch add-ons. Do you have a specific instance where changing the message would have helped you resolve a problem? If we were to change the message, I would prefer it point to a documentation page under numpy.org that can grow with time to include common cases where users ran into this problem. I think this would be preferable to a blanket suggestion to update numpy, which could break other things.

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 6, 2022

@mattip The reason I came here is that some coworkers got this error message and were lost about it. When googling this error, all the answers on StackOverflow tell to upgrade numpy version. This was a work around in our case but we didn't understand the reason so I decided to do some research and ended up here.

I agree with your suggestion of making the error message point to a documentation page under numpy.org . Do you have a suggestion of where this documentation would fit and what would be its content ? Is it open for PRs ?

@mattip
Copy link
Member

mattip commented Jul 6, 2022

We have the troubleshooting guide, the source is here. PRs are welcome. I think the date, the message, what two packages+versions caused the problem, and what version of NumPy provided a solution would be specific enough to be helpful.

@joaopfg
Copy link
Contributor Author

joaopfg commented Jul 8, 2022

@mattip @seberg I opened a PR for this one (#21943)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants