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

Working on adding curvature. #74

Merged
merged 4 commits into from Mar 30, 2013
Merged

Conversation

fryguybob
Copy link
Member

There are a couple of open questions with this still. One is how should we generalize the space that we compute at least the squared version. The second is what return values should we use. Would it be better to extend some scalar space with infinity explicitly?

Finally it would be nice to have this work for higher dimensions.

Also due to some failing systems here, I have not compiled this locally yet :D.

--
-----------------------------------------------------------------------------

module Diagrams.TwoD.Curvature
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be Diagrams.TwoD.Curvature?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, yes. This was a consequence of not actually compiling...

@byorgey
Copy link
Member

byorgey commented Feb 25, 2013

For representing a scalar space extended with positive infinity we already have Data.Monoid.PosInf from the monoid-extras package:

http://hackage.haskell.org/packages/archive/monoid-extras/0.2.2.2/doc/html/Data-Monoid-PosInf.html

Hmm, though it looks like it doesn't have a Num instance, which would probably be useful here, but that would be easy to add. Anyway, I am not saying this is definitely the way to go but it's worth thinking about. I don't know if there are other benefits to the (Double,Double) representation?

@fryguybob
Copy link
Member Author

I think most uses will want to pattern match right away, so I'm not sure a Num instance is important. Having (Double,Double) does not signal this intent very well and could easy be mistaken for something else. I think I will change over to PosInf and add a radiusOfCurvature version.

@byorgey
Copy link
Member

byorgey commented Mar 29, 2013

Is this ready to be merged? Or is there more work to do yet?

@fryguybob
Copy link
Member Author

Yeah it should be fine to merge now. If we find a good place to put the
derivatives that can of course be factored out.

On Fri, Mar 29, 2013 at 4:54 PM, Brent Yorgey notifications@github.comwrote:

Is this ready to be merged? Or is there more work to do yet?


Reply to this email directly or view it on GitHubhttps://github.com//pull/74#issuecomment-15660234
.

byorgey added a commit that referenced this pull request Mar 30, 2013
Working on adding curvature.
@byorgey byorgey merged commit 5c56431 into diagrams:master Mar 30, 2013
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.

None yet

2 participants