Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QuickCheck test failures #441

Open
sorki opened this issue Jun 20, 2023 · 2 comments
Open

QuickCheck test failures #441

sorki opened this issue Jun 20, 2023 · 2 comments

Comments

@sorki
Copy link
Contributor

sorki commented Jun 20, 2023

found on Hydra. Seems mostly related to (full|empty)Space

Failures:

  tests/ImplicitSpec.hs:303:3: 
  1) symbolic obj 3, identity, difference is complement
       Falsified (after 43 tests and 15 shrinks):
         [extrude (scale (V2 0.0 0.0) emptySpace) 0.0]
         (V3 0.0 0.0 0.0,())
         Inside /= Outside

  To rerun use: --match "/symbolic obj 3/identity/difference is complement/"

  tests/ImplicitSpec.hs:141:3: 
  2) symbolic obj 3, inverses, scale inverse
       Falsified (after 3 tests and 3 shrinks):
         V3 2.0 0.0 0.0
         (fullSpace,(V3 0.0 0.0 0.0,()))
         Outside /= Inside

  To rerun use: --match "/symbolic obj 3/inverses/scale inverse/"

  tests/ImplicitSpec.hs:250:3: 
  3) symbolic obj 3, 3d transform, scale
       Falsified (after 3 tests and 3 shrinks):
         V3 2.0 0.0 0.0
         (fullSpace,(V3 0.0 0.0 0.0,()))
         Inside /= Outside

  To rerun use: --match "/symbolic obj 3/3d transform/scale/"

Randomized with seed 1283303818

and

Failures:

  tests/ImplicitSpec.hs:141:3: 
  1) symbolic obj 2, inverses, scale inverse
       Falsified (after 2 tests and 3 shrinks):
         V2 0.0 (-1.0)
         (fullSpace,(V2 0.0 0.0,()))
         Outside /= Inside

  To rerun use: --match "/symbolic obj 2/inverses/scale inverse/"

  tests/ImplicitSpec.hs:141:3: 
  2) symbolic obj 3, inverses, scale inverse
       Falsified (after 1 test and 4 shrinks):
         V3 0.6249951862256663 0.0 0.0
         (cylinder 0.0 1.0,(V3 0.0 0.0 0.0,()))
         Outside /= Surface

  To rerun use: --match "/symbolic obj 3/inverses/scale inverse/"

  tests/ImplicitSpec.hs:221:3: 
  3) symbolic obj 3, 3d rotation, (x + y = 360) degrees is id
       Falsified (after 1 test and 6 shrinks):
         0.0
         V3 0.0 0.0 0.5991642884513831
         (cube False (V3 0.0 0.1 0.0),(V3 0.0 0.1 0.0,()))
         Outside /= Surface

  To rerun use: --match "/symbolic obj 3/3d rotation/(x + y = 360) degrees is id/"

Randomized with seed 563777087
@sorki sorki mentioned this issue Oct 26, 2023
@sorki
Copy link
Contributor Author

sorki commented Oct 30, 2023

This is caused by an improved Arbitrary Double instance which now generates exact 0 (and -0) values which aren't properly handled at AST level. Some primitives have a clause that tries to do that i.e. polygon [] = emptySpace, so it seems we can handle these at primitives level by adding for example square _ v | hasZeroComponent v = emptySpace but while the arbitrary instance uses square <*> arbitrary <$> arbitrary it actually translates to Square ... or Translate $ Square .. and when a property is violated, shrinking operates on the latter and can produce a zero sized Square.

We also cannot use NonZero from QuickCheck as that would require either

  • NonZero (Square (V3 Double)) that is not allowed due to Num a => NonZero a and no Num for Objects
  • or Square (NonZero (V3 Double)) which is not allowed by the defintion of Square

Another fun case is Scale which is generated directly (without using scale) and my idea was to switch it from

instance (Arbitrary obj, Arbitrary a, Arbitrary (f a), CoArbitrary (f a))·
  => Arbitrary (SharedObj obj f a) where
  shrink = genericShrink
  arbitrary = oneof
    [ Translate    <$> arbitrary    <*> decayArbitrary 2
    , Scale        <$> arbitrary    <*> decayArbitrary 2
....
    ]

to

instance (Arbitrary obj, Arbitrary a, Arbitrary (f a), CoArbitrary (f a)
 , Object (SharedObj obj f a) f a
 )
  => Arbitrary (SharedObj obj f a) where
  shrink = genericShrink
  arbitrary = oneof
    [ translate   <$> arbitrary    <*> decayArbitrary 2
      scale          <$> arbitrary    <*> decayArbitrary 2
...
    ]

by making a SharedObj an instance of Object as well by allowing SharedObj to be embedded into SharedObj using SharedNest

data SharedObj obj f a 
  = Empty  -- ^ The empty object
...
  | SharedNest (SharedObj obj f a)
  deriving (Generic, Show)

and a nasty instance

instance Object obj f a => Object (SharedObj obj f a) f a where
  _Shared :: Prism' (SharedObj obj f a) (SharedObj (SharedObj obj f a) f a)
  _Shared = prism' embed project
    where embed :: SharedObj (SharedObj obj f a) f a
                -> SharedObj obj f a
          embed x = unsafeCoerce @(SharedObj (SharedObj obj f a) f a) @(SharedObj obj f a) (SharedNest x)
          project = \case
            SharedNest y -> Just $
                unsafeCoerce @(SharedObj obj f a) @(SharedObj (SharedObj obj f a) f a) y
            _ -> Nothing
    --_Shared = prism' SharedNest (\case
    --    SharedNest y -> Just y
    --    _ -> Nothing
    --  )
  getBox x = getBox (Shared x)
  getImplicit' ctx x = getImplicit' ctx (Shared x)

While this works, it doesn't solve the actual problem which is described above using Square vs square.

Another fun discovery was that stuff that is failing in QC testsuite works when copied into repl because of custom Show instance which turns Square (V3 0 0) into square (V3 0 0) which is eliminated by added zero handling clause for the primitive. This can be improved by deriving ShowS so we have both a generic show and the custom pretty Show (i.e. using https://hackage.haskell.org/package/generic-deriving-1.14.5/docs/Generics-Deriving-Show.html)

Finally, the immediate solution seems to be adding the zero-guard-clauses directly to getBox(2|3)/getImplicit(2|3) functions so that Square (V3 0 0) gets short-circuited to Shared2 EmptySpace instead of exercising the underlying implicit function which may lead to NaNs and similar.

sorki added a commit to sorki/ImplicitCAD that referenced this issue Oct 31, 2023
Related to Haskell-Things#441

This will get factored out into canonicalization pass
sorki added a commit to sorki/ImplicitCAD that referenced this issue Oct 31, 2023
Related to Haskell-Things#441

This will get factored out into canonicalization pass
sorki added a commit to sorki/ImplicitCAD that referenced this issue Nov 1, 2023
Related to Haskell-Things#441

This will get factored out into canonicalization pass
@julialongtin
Copy link
Member

looks like we've fixed this.

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

No branches or pull requests

2 participants