goldfirere / units

The home of the units Haskell package
95 stars 19 forks source link

Add vector space operations based on the linear package #45

Closed hesiod closed 8 years ago

hesiod commented 9 years ago

Provide a Data.Metrology.Linear module based on Edward Kmett's linear package as an alternative to Data.Metrology.Vector based on the vector-space package. The cross product on two dimensional vectors has been dropped. Affine space data types (Points) are already defined in Linear.Affine and thus don't need to be defined here. I also dropped QPoints since I don't know enough about type families to update them. A possible solution could look like this:

type family QPoint f a where
  QPoint (Qu d l (f a)) = Qu d l (Point f a)

I didn't test the new module extensively, but there really shouldn't be any hidden surprises.

goldfirere commented 9 years ago

At a glance, this looks reasonable, and something I'd like to add. I would like to give all of this proper thought, though. As previously commented, I don't have the ability to do so right now. Does it ruin your day if I don't merge for a few weeks? Feel free to work from your branch, as I do expect to merge this (with perhaps a few tweaks, etc.) someday in the not-too-far-future.

Many thanks for your contributions!

A great thing that could come with this PR is some testing code. How do you plan on using this module? An example test file would be really helpful. I would also adapt that code to use Vector.

hesiod commented 9 years ago

No problem at all, I have no deadlines.

I'm developing a project using units, and I'm planning to make all vector operations use linear (for the simple reason that linear supports matrices out of the box). I'm currently fighting with OpenGL, but after that I can take some code from it and make it standalone.

goldfirere commented 9 years ago

I've created a branch linear with your commit and a big tweak from me. The goal of the tweak is that the linear interface need not be in lock-step with the vector-space interface. So I made the linear interface look more like the linear functions. Because linear doesn't try to "take over" normal arithmetic the way that vector-space does, I was able to remove all the re-exports.

The real problem here is that I really don't know enough about lenses or linear to do this module justice. I believe that, as written, it is unit-safe, but I'm not convinced it's useful! For example, I don't have equivalents for the R1...R4 classes. Is that OK? What about all the other functionality in linear?

Bottom line: I'm happy to have this functionality in units, but I feel ill-equipped to build it out and be confident in it. It might be reasonable to take the module we've written and spin in out into a new package. Anyway, take a look at what I've done and see how that goes for you. I'm going to leave this PR open for now, but I have no immediate plans to bring this in master. (Do feel free to try to convince me to, though!)

hesiod commented 8 years ago

Hi, it took some time, but I finally found time to have a look at this again.

You're absolutely right about building the linear interface on top of Poly. Actually, that is the main flaw of the vector-space interface: You can only have vector spaces as the underlying type. If you want to mix scalar units with vector units, you're going to end up having constraints like Fractional (Scalar (Scalar a)), and that certainly isn't the right way.

Looking at the other stuff right now.

PS: You forgot to include lens as a dependency in the cabal file.

hesiod commented 8 years ago

@goldfirere, if you merge #50 and #51, support for linear will be quite finished.

The R1..4 instances you mentioned, along all the other instances for V0..4 are nice to have, but not really essential - we all know ekmett's love for fancy category theory stuff, right? For example, R2 is just about having a Lens for the y component. I guess if someone really needs one of those, it won't be too difficult to implement the instance and send a pull request, but for now, units's support for linear is mostly complete. (There probably is some boilerplate-avoiding way to implement the instances using Generics, but I'm not quite sure how to do that.)

I would say that there's nothing to stop you from merging the linear branch. The only downside for users would be the dependency on lens which drags in quite some stuff.

If you want to see all this in action, my project xenocrat uses the linear branch (you need to do a cabal sandbox add-source /path/to/units). (It is not unlikely that it will refuse to run if your graphics card is too old and doesn't support OpenGL 4.5, but it should definitely type check, and that's obviously what matters)

goldfirere commented 8 years ago

OK. I've merged your other pull requests and sent this back to Travis. If Travis likes it, I'll mainline and release, trusting your judgment. Thanks for your contribution!

hesiod commented 8 years ago

Thanks for your time!

I don't have time to investigate this thoroughly at the moment, but it seems the Travis failure for GHC 7.10 is still due to #44 and the GHC bug you mentioned there.