Skip to content

Conversation

@carterbox
Copy link
Member

@carterbox carterbox commented Nov 12, 2025

This migrator adds the tegra variant to the existing arm_variant_type key in the global pinnings. The tegra variant is only for cuda_compiler_version 12.9 and has a different (higher) c_stdlib_version than the CUDA 12.9 migrator.

Modifications to the other CUDA migrations and the global pinnings were needed. These modifications including adding arm_variant_type to the global zip with the cuda_compiler_version when the platform is linux and the architecture is aarch64 and setting the default arm_variant_type to None.

The default arm_variant_type should be None and not sbsa because non-CUDA builds should be installable on both sbsa and tegra devices.

Testing this migrator in this feedstock: conda-forge/cusparselt-feedstock#72

@conda-forge-admin
Copy link
Contributor

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/meta.yaml) and found it was in an excellent condition.

@carterbox carterbox marked this pull request as ready for review November 14, 2025 22:43
@carterbox carterbox requested a review from a team as a code owner November 14, 2025 22:43
Copy link
Member

@isuruf isuruf left a comment

Choose a reason for hiding this comment

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

Besides the issue of zip_keys becoming larger as in the other issue, this doesn't work when a feedstock has a cpu build.

@carterbox
Copy link
Member Author

The migrator is now compatible with feedstocks that include a non-CUDA build variant.

The migrator now requires that both the CUDA 12.9 and CUDA 13.0 migrators have been added already.

@carterbox carterbox requested a review from isuruf November 17, 2025 16:57
Comment on lines 88 to +90
arm_variant_type: # [aarch64]
- sbsa # [aarch64]
- None # [aarch64]
- sbsa # [aarch64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
Copy link
Member Author

Choose a reason for hiding this comment

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

@jakirkham, I have changed the default arm_variant_type to None because non-CUDA builds don't care about arm_variant_type.

@carterbox
Copy link
Member Author

@conda-forge-admin, please restart ci

https://github.com/conda-forge/cuda-feedstock/blob/main/recipe/doc/recipe_guide.md#building-for-arm-tegra-devices
use_local: false
paused: true
Copy link
Member Author

Choose a reason for hiding this comment

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

This migrator is paused so that it is opt-in. Community feedback from robotics people is that there are probably many CUDA 12.9 packages that people would never run on a Tegra device, so it's not worth migrating the entire channel.

@carterbox
Copy link
Member Author

@conda-forge/core, please review.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

After making the suggested changes, could you please demonstrate a rerender on a branch (or in a test PR) of that migrator (together with the other changes to migrators and pinnings here, which will require use_local: true for cuda129/130 and -e to override the pinning file)? Say, on the libmagma feedstock.

check_solvable: false
wait_for_migrators:
- cuda129
- aarch64 and ppc64le addition
Copy link
Member

Choose a reason for hiding this comment

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

not sure if this works? I don't think it makes a lot of sense to wait for that migrator in practice (even though it's required conceptually) - the aarch/ppc migration is complete up to very few packages, and if someone wants to fix up aarch support for a feedstock that doesn't have it yet because they want tegra support, all the better.

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if this works?

It's unclear by what you mean. This is the correct name of the aarch64 migrator.
https://github.com/search?q=org%3Aconda-forge+%22aarch64+and+ppc64le+addition%22&type=code
https://github.com/search?q=repo%3Aregro%2Fcf-scripts+++%22aarch64+and+ppc64le+addition%22&type=code

I don't think it makes a lot of sense to wait for that migrator in practice (even though it's required conceptually) - the aarch/ppc migration is complete up to very few packages, and if someone wants to fix up aarch support for a feedstock that doesn't have it yet because they want tegra support, all the better.

In practice, I don't expect anyone to unpause this tegra migrator; all migrations will be manual. But if we do, then it is required that all the dependencies of a given package have been migrated to include aarch64 before tegra can be enabled. It doesn't prevent the use of this migrator to include this item in "wait_for_migrators", and it's technically correct.

Copy link
Member

Choose a reason for hiding this comment

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

It's unclear by what you mean. This is the correct name of the aarch64 migrator.

Scarred by spaces messing up "simple" string handling.

all migrations will be manual

That's a good point; it means the wait_for_migrators: is completely pointless (which is aimed at our bots, rather than at humans), but in that sense, it also doesn't hurt to have it in there, because it'll never be applied. 🤷

Comment on lines +42 to +46
c_compiler_version: # [linux and aarch64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 14 # [linux and aarch64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 14 # [linux and aarch64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 14 # [linux and aarch64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
- 14 # [linux and aarch64 and os.environ.get("CF_CUDA_ENABLED", "False") == "True"]
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be better to turn this into a operation: key_add migrator, rather than redefining existing pins.

Copy link
Member Author

@carterbox carterbox Dec 4, 2025

Choose a reason for hiding this comment

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

I tried for multiple days to create a correct migrator using operation: key_add instead of kind: version. The reoccurring problems were that a tegra variant for CUDA 13.0 would also be created and/or that the glibc version for variants would be incorrect or there would only be one CUDA 12.9 variant instead of two.

I need specific advice on how to accomplish this task or else I will not try again.

Copy link
Member

Choose a reason for hiding this comment

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

Migrators are applied in the order of their migrator_ts. In cases where the arithmetic depends on a specific sequence of application (e.g. because one migrator adds an additional_zip_keys: that all following migrators now need to populate), then you need to reflect that order in the timestamps (even if the dates don't correspond to reality). Don't change the timestamps of existing migrators though, only the one you're adding.

It's good to apply migration by migration on a feedstock in question (in the sense of testing a rerender with only the first one, then the second, then the third added to .ci_support/migrations) and see when/where things go wrong.

or there would only be one CUDA 12.9 variant instead of two.

What's the desired target state? Both CUDA 12.9 and 13.0 should get a tegra variant?

Copy link
Member Author

Choose a reason for hiding this comment

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

The following variants should exist for aarch64

cuda_compiler_version tegra
None None
12.9 sbsa
12.9 tegra
13.0 sbsa

There is no tegra variant for CUDA >=13

@carterbox
Copy link
Member Author

After making the suggested changes, could you please demonstrate a rerender on a branch (or in a test PR) of that migrator

I have updated my test branch with the latest changes. conda-forge/cusparselt-feedstock#72

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.

4 participants