Conversation
Micky774
left a comment
There was a problem hiding this comment.
Minor nits, and a couple questions regarding NV build behavior changes.
| # If the framework extension pip package is installed, it means that TE is installed via | ||
| # PyPI. For this case we need to make sure that the metapackage, the core lib, and framework | ||
| # extension are all installed via PyPI and have matching versions. | ||
| # Metapackage and core lib matchiong is checked in `sanity_checks_for_pypi_installation()`, |
There was a problem hiding this comment.
| # Metapackage and core lib matchiong is checked in `sanity_checks_for_pypi_installation()`, | |
| # Metapackage and core lib matching is checked in `sanity_checks_for_pypi_installation()`, |
| raise RuntimeError( | ||
| "Found empty `transformer-engine` meta package installed. " | ||
| "Install `transformer-engine` with framework extensions via" | ||
| "'pip3 install --no-build-isolation transformer-engine[rocm_pytorch,rocm_jax]'" |
There was a problem hiding this comment.
| "'pip3 install --no-build-isolation transformer-engine[rocm_pytorch,rocm_jax]'" | |
| " 'pip3 install --no-build-isolation transformer-engine[rocm_pytorch,rocm_jax]'" |
|
|
||
| # Name of the pip extra dependency for framework extensions from PyPI. | ||
| extra_dep_name = module_name | ||
| extra_dep_name = framework |
There was a problem hiding this comment.
This changes behavior for NV too, so let's guard it.
There was a problem hiding this comment.
Well, it it is apparent bug in upstream so I'd rather comment it but leave as-is
| f"`transformer-engine-{'rocm' if is_rocm else 'cu'}*`." ) | ||
|
|
||
| assert version(module_name) == version("transformer-engine") == te_core_version, ( | ||
| assert version(package_name) == te_core_version, ( |
There was a problem hiding this comment.
Does the NV build still need the version("transformer-engine") check?
There was a problem hiding this comment.
For NV meta package (transformer-engine) is mandatory. For ROCm it is made optional so we can install w/o naming conflict with PyPi where transformer-engine is NV.
There was a problem hiding this comment.
Sure but I just mean that upstream still retains the te version check here, so is it something redundant for even them or is it something that removing might impact expected behavior for an NV build?
There was a problem hiding this comment.
Oh, yes, It is indeed redundant: on 2.12 sanity_checks_for_pypi_installation checks that core and meta versions match at
|
Hi @ipanfilo , do you have a simple issue description like how this issue can be triggered, like a jira/github ticket? Not quite sure what your "meta package" refers to? rocm_pytorch, rocm_jax? |
Meta package refers for dummy transfer-engine wheel, the 4th (or the 1st) from the wheel set. Please refer https://github.com/ROCm/TransformerEngine/blob/dev/docs/installation.rst#pip---from-wheels , it tells about 2.10 changes on ROCm |
Description
Fix loading TE when install wheels w/o meta package
Type of change
Changes
Checklist: