Skip to content

Conversation

@alfredjmduncan
Copy link
Contributor

Replaces the pullback for axes argument with NoTangent().

@jonniedie
Copy link
Collaborator

Hey, so my knowledge of ChainRules is admittedly pretty low. Would you mind explaining why we wouldn't need the axes in the pullback? Or I guess what we get from using NoTangent here? Is this just a performance thing so we can eliminate what's likely to be a dead branch in the pullback? Or is this producing something incorrect?

@jonniedie
Copy link
Collaborator

Or is the idea that differentiating through Axis indices is not going to be meaningful anyway?

@alfredjmduncan
Copy link
Contributor Author

Or is the idea that differentiating through Axis indices is not going to be meaningful anyway?

That's right, apologies for the short PR description. As the Axis is discrete, no derivative exists for it. My original motivation was that I was running into an error using ComponentArrays with AdvancedVI.jl, where the existence of the pullback for the Axis was causing a problem. But my understanding is that the pullback for the axes argument should in theory be a NoTangent(). Tagging @oxinabox and @ChrisRackauckas as we had a brief exchange on Slack.

@jonniedie
Copy link
Collaborator

Okay cool. That makes sense. I’ll go ahead and merge it.

Thanks for the pr!

@jonniedie jonniedie merged commit 26ffcbc into SciML:master Aug 12, 2021
jonniedie added a commit that referenced this pull request Aug 12, 2021
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.

2 participants