-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Possible problem with Atom processors and conda-forge builds #67
Comments
Hm, this opened a little can of worms, I am afraid. The gist is that we shouldn't be optimizing with AVX unless RDKit gracefully falls back to a different implementation at runtime. If this cannot happen, we need to rebuild the packages without AVX, which might incur in a performance penalty for those users that could benefit from them. What do you think @greglandrum? Is there an easy way RDKit could handle that or do we need to rebuild? In the future, |
The issue isn't with AVX, it's [1] https://github.com/rdkit/rdkit/blob/bc4d5478478542045a0f89e28a5da8b6d1f0e5d0/CMakeLists.txt#L68-L76 |
Assuming that the problem @gokhantahil is actually due to the use of an AVX instruction (which we don't know for sure): As for what should be done on the conda-forge side: I'm really not sure. I guess this isn't a problem which comes up a lot (this is the first report I can remember of this type of problem), but at the same time I think conda-forge is intended to be a generally useful distribution... so maybe it doesn't make sense to be aggressive with the compile flags and to disable |
It seems a shame to cause a performance hit to the vast majority of users, given how widespread popcnt support is? I fall on the side of the conda-forge package catering to the most widespread use case, rather than the lowest common denominator. Especially seeing as we've lasted this long without issues and it seems archspec is coming soon to solve the problem? |
What about providing a |
We need the variant with and without |
@jaimergp : do you have any sense as to whether or not this is likely to be resolvable? |
The infrastructure for Basically, this requires a couple hours of work and I am currently super busy, but if this is one of those issues that are important but not urgent, I will tackle it eventually! |
I don't think it's particularly urgent. I asked because it came up again this week in rdkit/rdkit#4219 with someone using a really old Intel processor. |
In that case I'd suggest we wait for an official |
This issue filed in the RDKit repo seems to be connected with the conda-forge builds on Atom processors:
rdkit/rdkit#3814
I don't have access to an Atom processor to test on and I'm wondering if any of you has seen something like this before.
The text was updated successfully, but these errors were encountered: