Thanks a lot for the fix, I think it should be merged asap.

Should we then add tests for the different ways of configuring CUTLASS location?

Should we then add tests for the different ways of configuring CUTLASS location?

Having a test on CI that builds all operations and loads them, even without execution would catch things like this and e.g. #949
Not sure why this isn't done already, so I only assume there are reasons.

Should we then add tests for the different ways of configuring CUTLASS location?

Having a test on CI that builds all operations and loads them, even without execution would catch things like this and e.g. #949 Not sure why this isn't done already, so I only assume there are reasons.

Not necessarily, as there are ways to compile the extension without making CUTLASS_PATH point to a directory - this is the codepath we use in our side. I was testing all use cases locally before I introduced the simple mistake and clearly I became too confident and failed to consistently continue testing.

To make sure all works well, we should test all 3 ways of configuring CUTLASS_PATH: DS_IGNORE_CUTLASS_DETECTION, DS_USE_CUTLASS_PYTHON_BINDINGS and hardcoded paths. Right now compilation works for the first case. I personally would actually remove the other two and always ask the user to have a well setup compiler configuration, without deepspeed specific config. But that would be an intended breaking change.

Alternatively, we could vendor cutlass to provide a sane default that always works and allow knowledgeable users to redefine it, but I guess that is not something we want to do.

Mentioned in the other conversation (let's move here): As a workaround, would pointing to the relevant include dirs (e.g., using CPATH) and removing deepspeed specific configuration (i.e., setting CUTLASS_PATH=DS_IGNORE_CUTLASS_DETECTION) work for you?

Yes CUTLASS_PATH=DS_IGNORE_CUTLASS_DETECTION avoids the problematic code path and as CUTLASS is header-only CPATH works.

Hi @Flamefire, thank you for the fix, and thanks @sdvillal for the discussion.

Because this issue causes a real compilation break, I’m going to merge this PR now.

For follow-up, we should add coverage for Evoformer/CUTLASS build paths. Our per-PR CI currently runs a reduced unit-test set and does not compile Evoformer yet. We’re also working on restoring nightly full tests, and I think this coverage should be added there first.

Thanks @tohtana.

I think it would be best to remove all the custom configuration from the evoformer and report a changing break. Do we have other examples in deepspeed on how cutlass is handled?