Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
tsnecuda is now in conda-forge (irrespective of this PR, which we'll ignore for a moment) - I'd appreciate if you could try the following and let me know if things work as expected: (replacing If things are working as intended, then great! 🥳 If not, please let me know more or less urgently, so that we can fix the builds, or - in the worst case - mark them as broken so people cannot install them. Assuming things are more or less OK, this PR builds tsnecuda against faiss 1.7. It's not merged yet, i.e. cannot normally be installed through conda-forge. But to test it you could download one of the artefacts from this PR (choosing the python/CUDA version you want; if your cuda version is >11.2, take the CUDA=11.2 artefact), and test if the performance drop from CannyLab/tsne-cuda#98 is also replicated with the conda-builds. To do so, unpack the artefact and then do: Note that faiss & tsnecuda are built against generic blas/lapack interfaces, and you can change the underlying implementation in the environment by adding |
|
@h-vetinari Not sure if this is user error, or if it's another problem.... Error is: |
|
I've never seen such an error upon environment creation (resp. package metadata collection already): It's particularly unusual because the installation of the package gets tested successfully in CI. I guess the only pertinent difference is the presence/absence of the Any ideas @conda-forge/conda? |
|
@isuruf It's unrelated to this PR though, so I'll fix it in another one. |
…nda-forge-pinning 2021.12.02.17.12.32
|
So @DavidMChan, that was an unusual error that had happened, but it should be fixed now... 😅 Would you mind giving this another shot? I.e. testing the current packages, as well as the build against faiss 1.7 from this PR? PS. the "how-to" from the comment above is still correct, but the link for the artefacts has changed. |
|
Looks like the 1.6.5 version is working now! Great work on this, I really appreciate the help (I've been trying to get this building correctly for conda for a really long time, and I've never had any success). I'll push an update to the readme for recommended installation procedures. In terms of updating to 1.7.x, it works! and it does seem like this may have fixed the issues in CannyLab/tsne-cuda#98 - though I did notice that the version string in tsnecuda is incorrect (We generate it dynamically, but it might make sense to push a fix to the upstream tsnecuda for this, we could just hardcode the version string instead of trying to generate it dynamically). It's also pretty easy to push an upstream update which removes setuptools as a runtime dependency (we only use it for locating the relative path of the libtsnecuda.so file), so we could hack around that using |
|
Happy to hear it! :) Regarding the version string - I had to patch that because it was running into compilation issues, and while doing so, I noticed that the patch-level of the version wasn't being baked in correctly (presumably it was implicitly inserting a float, which cannot have two dots). I had asked you at the time if you could review the patches, but maybe that was too easily overlooked. You can see the patches within the repo, but I've now also pushed the branch upon which these patches are based. As I said in that comment, all these patches could use improvement, but it would be great if they could be included in the upstream repo eventually and therefore avoid the need to carry patches on the feedstock at all (for the third one, I'll probably switch to creating a |
|
Ah, and once we move to faiss 1.7, it would also be possible in principle to support windows. There might be some patches generated due to that (once I find the time, depending on how gnarly it is) which I'd happily upstream. |
|
PS: "Yes please!" to getting rid of |
|
Ping @DavidMChan, any comments/questions about the above (e.g. the patches re:changes for 3.0.2)? |
|
Ah - sorry about this! It's been finals week! I think that I merged what is required (as well as removed setuptools) in the following branch: https://github.com/CannyLab/tsne-cuda/tree/3.0.1-rc0 Let me know if that looks good, and I'll start the process to deploy this more broadly. |
Please don't worry about this then - I was just curious what the current status is. |
|
Gentle ping @DavidMChan, hope you got through the finals well. :) |
|
Hi @h-vetinari! Thanks! I pushed the upstream changes to the 3.0.1-RC0 branch - and I've been waiting for you to confirm that's all that you need before releasing.... I also think I replied to the other two patches - is there something else you need me to take a look at? |
|
Hey David, sorry, this got lost with the holidays on my end. This is green, so from my side there's nothing more that's necessary. |
Perhaps with the tiny exception of actually releasing |
|
Ok! Sorry about this, since it got lost for a sec, but you should be all good to go now! |
|
Awesome, thanks! Upgrade PR (#6) is merged, builds should be available in 2-3 hours. :) |
|
Next stop: windows builds. :) |
|
Hello, thank you very much for your hard work! |
|
There was some work on this in #7, but it stalled for reasons I cannot remember. More importantly, faiss is currently not maintained, and that feedstock needs a quite substantial overhaul. I'm not using faiss myself, so this would have to come from someone else (though I'm willing to help review PRs). |
|
I said 1.7.1, not 1.17. In conda-forge, |
|
You can also consider this: CannyLab/tsne-cuda#130 - which should allow you to build windows support from scratch (or at least is a preview version of what could work). |
4 participants
Towards CannyLab/tsne-cuda#98