Port to linear #212

cchalmers · 2014-08-29T14:05:33Z

Extra changes I made along the way:

get rid of unnecessary Backend constraints.
general Cartesian functions (unitX, scalingY etc.) depends on the classes (R1, R2, R3) so same functions can be used for R2 and R3
SizeSpec2D iso: spec2D :: Iso' (SizeSpec2D n) (Maybe n, Maybe n)
Traced instance for BoundingBox
halfTurn, quarterTurn added (similar to fullTurn)
fromOrthogonal, fromSymmetric added (similar to fromLinear)

Some things I'd like to change:

change view to boxEnvelope or something similar to prevent clash with lens
No Diagrams.Prelude.ThreeD. Have R3 functions in Diagrams.Prelude (in anticipation of projections)

I'm thinking of renaming type families to F for the container and A for the number type and have alias V t = F t (A t). The associated type variables would also change, so it's Point f a, Diagram b f a etc. This makes it more consistent with linear and the V type has the same behaviour. Any thoughts?

…rams-lib into generalize-double

bergey · 2014-08-29T14:47:13Z

I think we should keep using v and n for the type variables. The very generic f-for-functor, a-for-whatever types read well in the small types of linear, but I find complex types like QDiagram b v n a much easier to read with mnemonic type variables.

The main reason for Prelude.ThreeD was reusing names in 2D and 3D with monomorphic (or less-polymorphic) types. Notably unitX, translateX, and friends. If you've made those all polymorphic, and it doesn't require too many extra type sigs in user code, I'm fine having just one Prelude.

I'll try to look at the rest of your changes tonight or tomorrow.

jeffreyrosenbluth · 2014-08-29T16:56:46Z

This really looks very nice !
I think the changes to lib show the benefits of using linear more clearly than core.
@cchalmers Nice work!

jeffreyrosenbluth · 2014-08-29T16:58:23Z

src/Diagrams/TwoD/Curvature.hs

+
+sq :: Num a => a -> a
+sq x = x * x
+{-# INLINE sq #-}


Is this really beter than ... (V2 q (p * p))...

Guess not. Really did it for squaredCurvature but I can use (join (*)).

To prevent name clashes, rotateAbout (2D) is now rotateAround and reflectAbout (3D) is now reflectAcross.

cchalmers · 2014-08-29T21:58:40Z

@bergey Yeah, I think you're right about using v and n. I guess I just don't like the Vn type alias name.

I've merged the 3D prelude. There was an issue with the name clashes with rotateAbout and reflectAbout. The 2D rotateAbout is now rotateAround. The 3D reflectAbout is now reflectAcross. The idea is:

Around => Point
About => Line
Across => Plane
but I'm open to suggestions.

@jeffreyrosenbluth Thanks! :)

bergey · 2014-08-31T14:08:34Z

This is looking good. Did you accidentally delete Diagrams.Prelude? Where are fromOrthogonal and fromSymmetric defined?

jeffreyrosenbluth · 2014-08-31T15:42:20Z

One small thing. @cchalmers you un-did our stylish-haskell import format (which i happen to like).
Would you mind running it through stylish-haskell to fix it. I think @Mathnerd314 set up a configuration file for it.

cchalmers · 2014-08-31T19:38:56Z

@bergey Yes, I did accidentally delete the prelude :) fromOrthogonal and fromSymmetric are in Diagrams.Core.Transform.

@jeffreyrosenbluth Found the config file and changed them all back :)

I also added new types for Polar, Cylindrical and Spherical coordinates. I need types with all the classes and basis elements etc. so I can use them for my plotting library, so I thought I'd just add them here. I can take them out if you feel they're too bulky.

I've been thinking more about #202. My idea is all the Has classes are numerical lenses with RealFloat constraints. They're not always "true" lenses but they're useful and normally do what you expect. Maybe we could enforce _theta returns an Angle in the [0, tau) range, _phi an Angle in [0,pi] and _r ≥ 0?

The other classes: R1, R2, Radial, Cylinder etc. are lenses on the basis. They should always follow the lens laws and can be wrapped in E to make basis elements but can't be used to convert between coordinates with a different basis.

I'm not sure I'm happy with my solution of having two classes for everything. It feels a little cumbersome and potentially confusing.

bergey · 2014-09-03T02:27:09Z

I do want to keep discussing ways to make polar coordinates better. I think it would be better to make that a separate pull request / discussion, since this PR is already big and tricky. One thought I had about angles is to provide a pair of Getters that normalize to [0,tau) and (-pi,pi], for the common cases where that is expected.

byorgey · 2014-09-13T01:05:47Z

src/Diagrams/Align.hs

+                => (v n -> a -> Point v n) -> v n -> n -> a -> a
+alignBy'Default boundary v d a = moveOriginTo (lerp ((d + 1) / 2)
+                                                    (boundary v a)
+                                                    (boundary (negated v) a)


These two arguments seem switched ---does lerp work dually to alerp?

Yes: alerp a b t = lerp t b a. This should be mentioned in the notes. Spent a while debugging to find this was the culprit (it's not even mentioned in linear).

bergey · 2014-09-13T16:05:43Z

I created a branches called linear here and in -core, as the easiest way to get travis to test this branch, and test branches of other repos against it. I also merged master into linear.

byorgey · 2014-09-14T00:28:40Z

So should I continue reviewing this pull request? Or should I look at the linear branch instead?

byorgey · 2014-09-14T02:08:22Z

I'm liking what I'm seeing so far, though I don't think I yet have a good grasp of all the issues.

If we go the linear route it means we can use the Point type from linear and get rid of vector-space-points, right?

byorgey · 2014-09-14T02:14:24Z

src/Diagrams/BoundingBox.hs

-    --TODO: Depend on Enum Basis?
-    us = map fst $ decompose (zeroV :: V a)
+  env <- (appEnvelope . getEnvelope) a
+  let h = fmap env eye


Where is eye defined? I can't seem to find it in linear. I'd like to understand exactly how this code is working --- it seems that linear makes working with basis vectors etc. much nicer than with vector-space.

It's defined in Diagrams.Core.Transform as

eye :: (Rep v ~ E v, Representable v, Additive v, Num n) => v (v n) eye = tabulate $ \(E e) -> zero & e .~ 1

vector-space's HasBasis is essentially replaced by the more powerful Rep v ~ E v, Representable v which I much prefer.

Aha, I see. I like that too.
On Sep 14, 2014 1:22 AM, "Chris" notifications@github.com wrote:

In src/Diagrams/BoundingBox.hs:

boundingBox a = fromMaybeEmpty $ do

env <- appEnvelope $ getEnvelope a

let h = recompose $ map (\v -> (v, env $ basisValue v)) us

l = recompose $ map (\v -> (v, negate . env . negateV $ basisValue v)) us

return $ NonEmptyBoundingBox (P l, P h)

where

-- The units. Might not work if 0-components aren't reported.

--TODO: Depend on Enum Basis?

us = map fst $ decompose (zeroV :: V a)

env <- (appEnvelope . getEnvelope) a

let h = fmap env eye

It's defined in Diagrams.Core.Transform as

eye :: (Rep v ~ E v, Representable v, Additive v, Num n) => v (v n)
eye = tabulate $ (E e) -> zero & e .~ 1

vector-space's HasBasis is essentially replaced by the more powerful Rep
v ~ E v, Representable v which I much prefer.

—
Reply to this email directly or view it on GitHub
https://github.com/diagrams/diagrams-lib/pull/212/files#r17517301.

cchalmers · 2014-09-14T05:34:02Z

I'll start pushing to the linear branch.

@byorgey Yes, we could get rid of vector-space-points. The extra functions are now in Diagrams.Core.Points.

cchalmers added 18 commits August 18, 2014 16:30

Some progress.

2a40713

More progress.

4b8469a

Bounding box.

48cbd1d

Bounding box.

fe4892a

Merge branch 'generalize-double' of https://github.com/cchalmers/diag…

a70c9cf

…rams-lib into generalize-double

Starting on 2D and 3D modules.

552c822

Change VN to Vn.

a267069

Almost done.

71d5ada

Some Bergey sugestions. (and combinators)

9961036

Builds without vector-space.

3a5ff43

Update for new lenses.

e1894c6

Coordinate instances for linear types.

f237e03

Remove unused types.

79c9a2e

Use linear's classes for _x, _y, _z.

cf195d0

Uncomment default pragma.

c989031

Bring back prelude.

26582ff

lerp has arguments reversed in linear.

15751d7

Added (poor) Traced instances for Bounding box.

507309a

jeffreyrosenbluth reviewed Aug 29, 2014
View reviewed changes

cchalmers added 2 commits August 29, 2014 18:45

General cleanup.

46a5a9e

Merge Prelude.ThreeD with Prelude.

317c7ec

To prevent name clashes, rotateAbout (2D) is now rotateAround and reflectAbout (3D) is now reflectAcross.

cchalmers added 3 commits August 31, 2014 17:09

Added Prelude back.

d4a5fbd

Added Polar, Cylindrical and Spherical coordinates.

5d68ad7

Use stylish-haskell config.

7e5ca2b

Fixed scaling measures.

e24baef

cchalmers added 2 commits September 4, 2014 00:09

Fix bug with showOrigin.

a11a0f3

Postpone new polar coordinate and general cleanup.

8457055

byorgey reviewed Sep 13, 2014
View reviewed changes

bergey mentioned this pull request Sep 13, 2014

update for linear diagrams/diagrams-contrib#40

Merged

byorgey reviewed Sep 14, 2014
View reviewed changes

cchalmers mentioned this pull request Sep 14, 2014

Linear #215

Merged

cchalmers closed this Sep 14, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to linear #212

Port to linear #212

cchalmers commented Aug 29, 2014

bergey commented Aug 29, 2014

jeffreyrosenbluth commented Aug 29, 2014

jeffreyrosenbluth Aug 29, 2014

cchalmers Aug 29, 2014

cchalmers commented Aug 29, 2014

bergey commented Aug 31, 2014

jeffreyrosenbluth commented Aug 31, 2014

cchalmers commented Aug 31, 2014

bergey commented Sep 3, 2014

byorgey Sep 13, 2014

cchalmers Sep 14, 2014

bergey commented Sep 13, 2014

byorgey commented Sep 14, 2014

byorgey commented Sep 14, 2014

byorgey Sep 14, 2014

cchalmers Sep 14, 2014

byorgey Sep 14, 2014

cchalmers commented Sep 14, 2014

Port to linear #212

Port to linear #212

Conversation

cchalmers commented Aug 29, 2014

bergey commented Aug 29, 2014

jeffreyrosenbluth commented Aug 29, 2014

jeffreyrosenbluth Aug 29, 2014

Choose a reason for hiding this comment

cchalmers Aug 29, 2014

Choose a reason for hiding this comment

cchalmers commented Aug 29, 2014

bergey commented Aug 31, 2014

jeffreyrosenbluth commented Aug 31, 2014

cchalmers commented Aug 31, 2014

bergey commented Sep 3, 2014

byorgey Sep 13, 2014

Choose a reason for hiding this comment

cchalmers Sep 14, 2014

Choose a reason for hiding this comment

bergey commented Sep 13, 2014

byorgey commented Sep 14, 2014

byorgey commented Sep 14, 2014

byorgey Sep 14, 2014

Choose a reason for hiding this comment

cchalmers Sep 14, 2014

Choose a reason for hiding this comment

byorgey Sep 14, 2014

Choose a reason for hiding this comment

cchalmers commented Sep 14, 2014