Skip to content

Conversation

@Pangoraw
Copy link
Collaborator

@Pangoraw Pangoraw commented Nov 4, 2024

Depends on EnzymeAD/Enzyme#2142.

@mofeing
Copy link
Collaborator

mofeing commented Nov 4, 2024

I'm dubious if we should go this way? AFAIK Jax doesn't do this unrolling. Like batching over a lot of operations should still be done without unrolling.

Do you know for which ops should the unrolling be done?

@Pangoraw
Copy link
Collaborator Author

Pangoraw commented Nov 4, 2024

Another possible generic strategy (maybe as a fallback) is to wrap the op in a stablehlo.map but there are no derivative rules for map yet.

@Pangoraw
Copy link
Collaborator Author

Pangoraw commented Nov 4, 2024

Here I have enabled the unrolling for just a few ops where the batch interface is needed and not implemented.

@Pangoraw Pangoraw changed the title Generic unrolling for batch op interface Generic batch op interface Nov 7, 2024
auto iszero = matchPattern(ifOp.getPred(), m_Zero());
auto isone = matchPattern(ifOp.getPred(), m_One());

if (!iszero && !isone)
Copy link
Member

Choose a reason for hiding this comment

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

can we also test if the input operand would be a broadcastop of a single value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added it but I am having a hard time coming up with an input to test this where the batched version of the predicate is a broadcast_in_dim.

Copy link
Member

Choose a reason for hiding this comment

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

yeah I think long term this will require an extension to the batching interface to mark some operands as non-batched, in any case is fine for now

@wsmoses wsmoses closed this Nov 7, 2024
@wsmoses wsmoses reopened this Nov 7, 2024
@wsmoses wsmoses merged commit 6440770 into EnzymeAD:main Nov 8, 2024
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.

3 participants