Conversation
Micky774
requested review from
ipanfilo,
wangye805 and
wenchenvincent
as code owners
| @@ -56,24 +56,39 @@ list(JOIN V3_ASM_ARCHS ";" V3_ASM_ARCHS_STR) | |||
|
|
|||
| if(DEFINED AITER_MHA_PATH) | |||
There was a problem hiding this comment.
Have we tested all possible paths (like with the aiter prebuilt env, NVTE_FUSED_ATTN_CK_PATH, and so on)?
There was a problem hiding this comment.
No not completely. I tested locally with a source build, but still need to test w/ the pre-built. The NVTE_FUSED_ATTN_CK_PATH path is tested on subsequent re-builds where we can skip the initial qola build, so that's tested too indirectly.
I did just push some changes to QoLA and the TE integration, so I'll re-test via source build and then run an aiter-prebuilt flow for it
There was a problem hiding this comment.
Let's do a thorough tests for those flows since ci will only test the default flow. Thanks
|
Tracking AITER pre-built upload here and will run local tests on the pre-built path when finished. |
ipanfilo
left a comment
ipanfilo
left a comment
There was a problem hiding this comment.
With QoLA AITER, 3rdparty/aiter has to be removed unless we plan to pass AITER location instead of AITER commit to QoLA
| --install-dir ${__AITER_MHA_PATH} | ||
| --gpu-archs "${V3_ASM_ARCHS_STR}" | ||
| --ck-tile-bf16 ${CK_FUSED_ATTN_FLOAT_TO_BFLOAT16_DEFAULT} | ||
| COMMAND ${CMAKE_COMMAND} -E env "PYTHONPATH=${__QOLA_DIR}:$ENV{PYTHONPATH}" |
There was a problem hiding this comment.
Should it rather include it as subproject? COMMAND was used for aiter_build because it is shell script
| target_link_directories(ck_fused_attn PUBLIC ${__AITER_MHA_PATH}) | ||
| set(__QOLA_TE_NAMESPACE_PREFIX "te_") | ||
| set(__QOLA_MHA_FWD_LIB "${__QOLA_TE_NAMESPACE_PREFIX}libmha_fwd.so") | ||
| set(__QOLA_MHA_BWD_LIB "${__QOLA_TE_NAMESPACE_PREFIX}libmha_bwd.so") |
There was a problem hiding this comment.
Other places use hardcoded te_libmha_* so let;s be consistent and do the same here
There was a problem hiding this comment.
@wangye805 I'm inclined to agree with Ilya here unless we expect or intend to support a pattern of consumers utilizing multiple conflicting versions of TE simultaneously.
There was a problem hiding this comment.
Yeah, I'm okay with both
|
Failure seems unrelated btw |
|
Current CI on this PR is source-build. CI run w/ pre-built download: https://github.com/ROCm/TransformerEngine/actions/runs/24856277623 |
3 participants
Description
This PR integrates QoLA as our main mechanism of building AITER, slotting it into existing infrastructure such as the aiter-prebuilt cache system.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
AITER_ASM_DIRmachinery in TEaiter::namespace for a macro-drivenQOLA_NS(*)wrapperChecklist: