[AutoDiff][Sema] Properly type-check differentiability of parameters and results in differentiable function type declarations #41174

philipturner · 2022-02-03T02:54:56Z

Resolve ambiguities between lack of a return type and lack of a differentiable return type. If a function returns Void, it returns no values and therefore returns no differentiable values. This rule allows type checking to mirror how it processes conventional functions, despite Void being differentiable.

// Before this PR, the following declaration might crash the compiler.
typealias FuncType = @differentiable(reverse) (inout Float, Float) -> Void

// While the following function has the same signature and compiles just fine.
@differentiable(reverse)
func foo(x: inout Float, y: Float) {
  x += y
}

Resolves three issues, listed below. The SR-15818 regression test was modified to avoid triggering SR-15823, an unrelated compiler + runtime crasher.

Resolves (partially): [SR-14021] Functions accepting multiple inout parameters aren't differentiable #56412
Resolves: [SR-15808] [AutoDiff] Non-differentiable closure parameters not parsed correctly #58085
Resolves: [SR-15818] [AutoDiff] Compiler crash for inout @noDerivative closure parameter #58095

philipturner · 2022-02-03T02:55:15Z

CC: @AnthonyLatsis

philipturner · 2022-02-03T02:56:05Z

#41128

AnthonyLatsis

Are you planning to add some tests? I am no AutoDiff expert, so I can only speak for basic stuff.

AnthonyLatsis · 2022-02-03T03:25:02Z

lib/AST/Type.cpp

@@ -6021,8 +6021,7 @@ TypeBase::getAutoDiffTangentSpace(LookupConformanceFn lookupConformance) {
      newElts.push_back(elt.getWithType(eltSpace->getType()));
    }
    if (newElts.empty())
-      return cache(
-          TangentSpace::getTuple(ctx.TheEmptyTupleType->castTo<TupleType>()));


We may want to add a "non-empty tuple" assertion to TangentSpace::getTuple to prevent this from happening elsewhere, if this is indeed correct.

That sounds like a good idea. I’ll change that before this PR gets merged.

philipturner · 2022-02-03T03:36:32Z

Are you planning to add some tests? I am no AutoDiff expert, so I can only speak for basic stuff.

You’re right! I should add tests! All the crashers (and the incorrect error diagnostic) I cited in SR-15808 should be tested.

Fixing the incorrect error diagnostic will probably require more changes to the C++ code, but within the same function as some other changes. I’m modifying how a differentiable closure is type-checked.

AnthonyLatsis · 2022-02-03T15:00:35Z

lib/AST/AutoDiff.cpp

+TangentSpace TangentSpace::getTuple(TupleType *tupleTy) {
+  if (tupleTy->getElements().size() == 0) {
+    llvm::report_fatal_error("Attempted to get tangent space of empty tuple.");
+  }


Note: We mainly use assert to stand guard of internal invariants. llvm::report_fatal_error is meant for something the user is to blame for, like passing a flag and violating its contract.

AnthonyLatsis · 2022-02-03T15:05:44Z

lib/AST/AutoDiff.cpp

@@ -366,6 +366,13 @@ GenericSignature autodiff::getDifferentiabilityWitnessGenericSignature(
  return derivativeGenSig;
 }

+TangentSpace TangentSpace::getTuple(TupleType *tupleTy) {
+  if (tupleTy->getElements().size() == 0) {


getNumElements is more efficient. Or you could do tupleTy->isVoid(), which does the check for you.

philipturner · 2022-02-03T18:46:56Z

The inconsistent error diagnostic requires attention in a separate pull request. It should automatically consider non-differentiable stuff as like @noDerivative. But, when I bypass that check, it crashes in a later compilation stage. This restriction was never outlined in the Differentiable Programming Manifesto.

Also, there's a runtime crash here, where type checking should have prevented it from compiling. This runs successfully at runtime:

typealias MyType = @differentiable(reverse) (inout Float, @noDerivative Float, @noDerivative Int) -> Void

@differentiable(reverse)
func myFunc(_ x:  inout Float, _ q: @noDerivative Float, _ y: @noDerivative Int) -> Void {
  x
}
print(myFunc as MyType)

While this crashes, by adding an extra line that does nothing:

typealias MyType = @differentiable(reverse) (inout Float, @noDerivative Float, @noDerivative Int) -> Void

@differentiable(reverse)
func myFunc(_ x:  inout Float, _ q: @noDerivative Float, _ y: @noDerivative Int) -> Void {
  x
}
typealias MyType2 = @differentiable(reverse) (inout Float, @noDerivative Float, @noDerivative Int) -> Void
print(myFunc as MyType)

I got this crash by narrowing down the wierd error diagnostic, and I think they're related.

AnthonyLatsis · 2022-02-03T19:10:34Z

This one-liner reduced from your example crashes during type-checking with my build:

import _Differentiation

@differentiable(reverse) func myFunc(_ x:  inout Float) -> Void {}

philipturner · 2022-02-03T20:15:19Z

I don't see the crash happening. Did you build from my branch, or from a previous toolchain? The comment above was tested on the January 9, 2022 toolchain, not this branch.

The whole AutoDiff test suite passes on my branch as well.

AnthonyLatsis · 2022-02-03T20:54:08Z

Did you build from my branch, or from a previous toolchain?

No, this is a 2-day-old trunk.

philipturner · 2022-02-03T20:55:05Z

Please try branch then. Or, manually insert my changes onto main (there aren't that many).

CodaFi · 2022-02-03T21:00:55Z

Void should never have been differentiable in the first place.

Is this true? It's odd, yet a reasonable thing to take the derivative of the following enum type

enum Top: Differentiable {
  case one
}

Which is isomorphic to Void. Are we missing a builtin conformance to Differentiable?

philipturner · 2022-02-03T21:01:56Z

Top doesn't have a tangent vector. Try using it as the only input and output of a @differentiable function.

CodaFi · 2022-02-03T21:29:03Z

Top doesn't have a tangent vector.

Yes it does. In fact, it's (), an (arbitrary) representation of a zero-dimensional vector.

Try using it as the only input and output of a @differentiable function.

There is only one such function and it is differentiable (in the mathematical sense) because it's necessarily constant. Consider that the definition of differentiation involves quantifying over points not equal to whatever () is represented by in your equivalent manifold. There... aren't any of those.

philipturner · 2022-02-03T21:29:51Z

Then how else do you suggest we resolve this bug?

philipturner · 2022-02-03T21:31:21Z

Swift classifies @differentiable (Void) -> Void as an error.

CodaFi · 2022-02-03T21:33:50Z

This seems like a policy choice. Do you know what Julia or PyTorch do here?

philipturner · 2022-02-03T21:36:01Z

I don't think that's relevant if the Swift compiler already has treated it as non-differentiable for years. All unit tests pass with the branch how it is, without modifications. Also, you could manually conform your enumeration to Differentiable. Doing that for Void is a compile failure because tuples can't conform to protocols. Furthermore, if Void was truly differentiable, every differentiable function that returns nothing and has an inout parameter would return two semantic results (a compile failure).

CodaFi · 2022-02-03T22:10:39Z

because tuples can't conform to protocols.

We have begun to build the machinery for this with BuiltinProtocolConformance. It's okay for us to declare this case as unsupported, but there needs to be a diagnostic for it - I already see one for differentiable enums.

Furthermore, if Void was truly differentiable, every differentiable function that returns nothing and has an inout parameter would return two semantic results (a compile failure).

That's a solid point, but autodiff already rejects functions with multiple result types like this. I'm not asking for that to change, rather to consider that the reasoning behind us rejecting differentiable functions in R^{0} has to be stronger than looking at the implementation and declaring it moot, or merely getting the tests to pass.

philipturner · 2022-02-03T22:21:16Z

there needs to be a diagnostic for it - I already see one for differentiable enums.

Could you give me more context on how we might implement a special diagnostic for Void? For example, would we say something like:

Differentiation of R^{0} types coming in a future version of Swift

CodaFi · 2022-02-11T22:57:48Z

please elaborate on the best way to address classifying Void as differentiable.

I think we can support it given the machinery in the compiler today. You'll need to extend a builtin conformance to these types. It's a lot of plumbing, but taking a look at how we do this for Sendable should give you a feel for how to do this for non-nominal types in general.

philipturner · 2022-02-17T01:43:51Z

There's a way to solve these crashes without making Void un-differentiable.

philipturner · 2022-06-14T19:28:55Z

#56412 might refer to #38781, which is why I am not marking that issue as fully resolved.

* a commit * another commit * Update lib/Sema/TypeCheckType.cpp * Update lib/Sema/TypeCheckType.cpp

AnthonyLatsis reviewed Feb 3, 2022

View reviewed changes

philipturner mentioned this pull request Feb 4, 2022

AST: Remove legacy GSB-based GenericSignature query implementation #40812

Merged

philipturner changed the title ~~[AutoDiff] Make Void no longer classified as differentiable~~ [AutoDiff] Consistently classify Void as non-differentiable Feb 5, 2022

This comment was marked as off-topic.

Sign in to view

philipturner closed this Feb 17, 2022

philipturner mentioned this pull request Feb 17, 2022

[AutoDiff][Sema] Correctly process differentiable function type declarations with inout or Void #41422

Closed

philipturner reopened this Apr 21, 2022

philipturner marked this pull request as draft April 21, 2022 01:00

philipturner force-pushed the fix-sr15808 branch from 6d66e8a to d2f5188 Compare April 21, 2022 01:03

philipturner changed the title ~~[AutoDiff] Consistently classify Void as non-differentiable~~ [AutoDiff][Sema] Correctly process differentiable function type declarations with inout or Void Apr 21, 2022

philipturner changed the title ~~[AutoDiff][Sema] Correctly process differentiable function type declarations with inout or Void~~ [AutoDiff][Sema] Properly type-check differentiability of parameters and results in differentiable function type declarations Apr 21, 2022

philipturner marked this pull request as ready for review April 24, 2022 06:20

This was referenced Feb 3, 2022

[SR-15808] [AutoDiff] Non-differentiable closure parameters not parsed correctly #58085

Closed

[SR-15818] [AutoDiff] Compiler crash for inout @noDerivative closure parameter #58095

Closed

asl added the AutoDiff label May 3, 2022

philipturner requested a review from AnthonyLatsis May 4, 2022 21:39

philipturner mentioned this pull request May 8, 2022

[SR-14021] Functions accepting multiple inout parameters aren't differentiable #56412

Open

philipturner mentioned this pull request Jun 14, 2022

[AutoDiff][SIL] Reverse-mode differentiation: thunk generation crash #59429

Open

philipturner and others added 12 commits June 14, 2022 16:49

Debugging branch (#7)

0b265b0

* a commit * another commit * Update lib/Sema/TypeCheckType.cpp * Update lib/Sema/TypeCheckType.cpp

Can't get tangent space of void anymore

b64c7dc

Fix typo

f293cee

Fix some stuff

9dbe874

Clean up conditional

a5691a6

Add real diagnostic

ed830e8

Fix whitespace issues

d5f1bee

Add test 1/2

566e552

Add test 2/2

f465766

Fix SR-15818 and add test

17f43eb

Migrate changes from PR swiftlang#41422

c11f5ce

Modify spelling and whitespace

4b8312d

philipturner force-pushed the fix-sr15808 branch from 78a5b82 to 4b8312d Compare June 14, 2022 20:49

philipturner closed this Aug 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AutoDiff][Sema] Properly type-check differentiability of parameters and results in differentiable function type declarations #41174

[AutoDiff][Sema] Properly type-check differentiability of parameters and results in differentiable function type declarations #41174

philipturner commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022

AnthonyLatsis left a comment

AnthonyLatsis Feb 3, 2022 •

edited

Loading

philipturner Feb 3, 2022

philipturner Feb 3, 2022

philipturner commented Feb 3, 2022 •

edited

Loading

AnthonyLatsis Feb 3, 2022

AnthonyLatsis Feb 3, 2022

philipturner commented Feb 3, 2022

AnthonyLatsis commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

AnthonyLatsis commented Feb 3, 2022

philipturner commented Feb 3, 2022

CodaFi commented Feb 3, 2022

philipturner commented Feb 3, 2022 •

edited

Loading

CodaFi commented Feb 3, 2022

philipturner commented Feb 3, 2022

philipturner commented Feb 3, 2022

CodaFi commented Feb 3, 2022

philipturner commented Feb 3, 2022

CodaFi commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

This comment was marked as off-topic.

CodaFi commented Feb 11, 2022

philipturner commented Feb 17, 2022

philipturner commented Jun 14, 2022

[AutoDiff][Sema] Properly type-check differentiability of parameters and results in differentiable function type declarations #41174

[AutoDiff][Sema] Properly type-check differentiability of parameters and results in differentiable function type declarations #41174

Conversation

philipturner commented Feb 3, 2022 • edited Loading

philipturner commented Feb 3, 2022 • edited Loading

philipturner commented Feb 3, 2022

AnthonyLatsis left a comment

Choose a reason for hiding this comment

AnthonyLatsis Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

philipturner Feb 3, 2022

Choose a reason for hiding this comment

philipturner Feb 3, 2022

Choose a reason for hiding this comment

philipturner commented Feb 3, 2022 • edited Loading

AnthonyLatsis Feb 3, 2022

Choose a reason for hiding this comment

AnthonyLatsis Feb 3, 2022

Choose a reason for hiding this comment

philipturner commented Feb 3, 2022

AnthonyLatsis commented Feb 3, 2022 • edited Loading

philipturner commented Feb 3, 2022 • edited Loading

AnthonyLatsis commented Feb 3, 2022

philipturner commented Feb 3, 2022

CodaFi commented Feb 3, 2022

philipturner commented Feb 3, 2022 • edited Loading

CodaFi commented Feb 3, 2022

philipturner commented Feb 3, 2022

philipturner commented Feb 3, 2022

CodaFi commented Feb 3, 2022

philipturner commented Feb 3, 2022

CodaFi commented Feb 3, 2022 • edited Loading

philipturner commented Feb 3, 2022 • edited Loading

This comment was marked as off-topic.

CodaFi commented Feb 11, 2022

philipturner commented Feb 17, 2022

philipturner commented Jun 14, 2022

philipturner commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

AnthonyLatsis Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

AnthonyLatsis commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading

CodaFi commented Feb 3, 2022 •

edited

Loading

philipturner commented Feb 3, 2022 •

edited

Loading