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

Arch Migrator #58

Merged

Conversation

regro-cf-autotick-bot
Copy link
Contributor

@regro-cf-autotick-bot regro-cf-autotick-bot commented Nov 18, 2022

This feedstock is being rebuilt as part of the aarch64/ppc64le migration.

Feel free to merge the PR if CI is all green, but please don't close it
without reaching out the the ARM migrators first at @conda-forge/arm-arch.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/autotick-bot/actions/runs/3495928881, please use this URL for debugging.

@conda-forge-linter
Copy link

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 (recipe) and found it was in an excellent condition.

@jakirkham
Copy link
Member

@conda-forge-admin, please re-render

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@jakirkham jakirkham force-pushed the bot-pr_arch_h4d93ba branch from 4bf398a to eb76b40 Compare January 4, 2023 22:27
@conda-forge-linter
Copy link

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 (recipe) and found it was in an excellent condition.

@jakirkham
Copy link
Member

@conda-forge-admin, please re-render

@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2023

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/faiss-split-feedstock/actions/runs/3842104858.

@jakirkham jakirkham force-pushed the bot-pr_arch_h4d93ba branch from 85e32f5 to fe14caf Compare January 4, 2023 22:46
@jakirkham
Copy link
Member

@conda-forge-admin, please re-render

-B _build_python_avx2 \
faiss/python
cmake --build _build_python_avx2 --target swigfaiss_avx2 -j $CPU_COUNT
if [[ "${target_platform}" == *-64 ]]; then
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to match win-64 and linux-64 but not linux-aarch64 or linux-ppc64le? What about osx-64? I saw a comment below, the linux & windows CI agents support AVX2 (OSX doesn't yet). I'd prefer to check explicitly for specific matching strings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used on Windows. There are .bat build files for that.

AFAICT this was already building on macOS. So this doesn't change that.

All this does is exclude non-x86_64 targets.

@jakirkham jakirkham changed the title Arch Migrator Arch Migrator w/CUDA Jan 4, 2023
@jakirkham
Copy link
Member

cc @Ethyling

@h-vetinari
Copy link
Member

@jakirkham, you might find some useful (if somewhat dated) commits for this in #45

@jakirkham
Copy link
Member

Thanks Axel 🙏

Have looked through that PR and it looks like this includes similar changes. The approach differs slightly (though the differences are largely cosmetic)

@jakirkham
Copy link
Member

@conda-forge-admin, please restart CI

(Travis CI is having some issues cloning the PR for some reason. Hoping this clears that out)

@jakirkham jakirkham force-pushed the bot-pr_arch_h4d93ba branch from 0f46165 to 6926c55 Compare January 5, 2023 23:44
@jakirkham jakirkham changed the title Arch Migrator w/CUDA Arch Migrator Jan 5, 2023
@jakirkham
Copy link
Member

The CUDA builds are taking longer than the CI limits. Had briefly explored other ways to improve this (using Ninja), but ran into some bugs (likely due to the CMake config from FAISS).

As the other archs (CPU only) already build ok, have reverted to just the changes to build those for now. That way we can hopefully get those in sooner.

There will be another bot PR for the CUDA arch migrator. We can follow up on other changes needed for that one.

@jakirkham
Copy link
Member

It looks like all the jobs passed except one Linux x86_64 CUDA job that ran into the CI time limit. Guessing this just gets close and in this one case went over (based on looking at other similar jobs).

If the changes here seem reasonable, would suggest we merge and then I can babysit CI on main tomorrow to get the packages out.

@h-vetinari
Copy link
Member

If the changes here seem reasonable, would suggest we merge and then I can babysit CI on main tomorrow to get the packages out.

Yeah, that's what's unfortunately necessary on this feedstock. There were some specific CUDA builds that always timed out, which have consequently been skipped. It's a pity, with an hour or so more we'd be fine here. Or if we made some progress on archspec, we wouldn't have to compile the code twice for x86 to have fat binaries for sse/avx2...

@h-vetinari h-vetinari merged commit 1c7f899 into conda-forge:main Jan 6, 2023
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the bot-pr_arch_h4d93ba branch January 6, 2023 13:20
@jakirkham
Copy link
Member

Thanks Axel! 🙏

Yeah no worries.

Interesting about the archspec and repeated compilation point. Maybe this is an angle worth looking at further. Have there been existing attempts here?

@jakirkham jakirkham mentioned this pull request Jan 6, 2023
@h-vetinari
Copy link
Member

Interesting about the archspec and repeated compilation point. Maybe this is an angle worth looking at further. Have there been existing attempts here?

Have a look in #23 and the links therein. This is definitely worth pursuing, but conda-forge infra isn't ready yet, and I've given up pinging on the respective issue (which isn't on this feedstock).

@jakirkham
Copy link
Member

Thanks Axel! 🙏

Will take a look. Think one of the issues was we lacked the kind of plugin infrastructure we would want to start customizing Conda in the ways we would like. This was very recently introduced late last year. Please see this blogpost.

It may be more doable in the future, but would require more work in the tooling as well before conda-forge would see direct benefits.


On a different note, only a couple builds failed. Restarted them somewhat recently.


Have also refreshed the macOS ARM builds to use the changes from here simplifying that PR ( #39 ).


In terms of CUDA arch builds, PR ( #62 ) was recently opened by the migrator.

@jakirkham jakirkham mentioned this pull request Nov 28, 2023
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

Successfully merging this pull request may close these issues.

5 participants