Miscellaneous improvements

pthariensflame commented 10 years ago

Various improvements to Interval:
- Added ival' and ispan.
- Corrected Show instance to always produce valid Haskell code.
- Added valid Read instance in correspondence with the new Show instance.
- Corrected Eq instance so that it behaves mathematically sensibly.
- Replaced broken Ord instance with an instance for PartialOrd.
- Added logfloat as a dependency to support this.
- Re-exported PartialOrd and comparingPO to make this easy to use.
- Added Bounded instance.
- Added IntervalNesting newtype, supporting an alternative PartialOrd instance around interval nesting.
Implemented encodeFloat and decodeFloat (such as they are) for Dif.

DanBurton commented 10 years ago

Thanks for the pull request! I'm going to put my critical cap on, challenge a few of these design decisions, and ask some more people to review these changes.

DanBurton commented 10 years ago

I've started a discussion on Reddit.

http://www.reddit.com/r/haskell/comments/1whu8s/request_for_review_numbers/

cartazio commented 10 years ago

i'd advocate log-domain rather than log-float if you really wanted to add log domain floats. (which i don't really see being used, its just picked up because it has a partial-ord class, which i'd be inclined to be against. Unless you're saying its the canonical choice of hypothetical partial ord class)

likewise, i'm not sure if you gain much by adding a partialOrd type class, a simple partialCompare function for intervals would be just fine!

removing the "slightly illegal" instances, like Ord, while "valid", we have to accept that base has "slightly illegal" instances too,

cartazio commented 10 years ago

in case its not clear, I really don't like log-float, i like log-domain is a better lib :)

cartazio commented 10 years ago

just to be clear: i do favor improving numbers, but if we're going to deal with illegal instances, we should work on a whole new number prelude! Working around the flaws in the stand number prelude in a localized way just won't buy you that much.

pthariensflame commented 10 years ago

I don't think just a partialCompare function would cut it. I think a full PartialOrd type class is absolutely necessary; I don't really care where it comes from, but I would rather use an existing one than add yet another copy to numbers. If you can find a package other than logfloat that implements a full PartialOrd type class, I would be happy to take a look at it.

DanBurton commented 10 years ago

@cartazio, regarding "we should work on a whole new number prelude!", that's been done numerous times (pun intended), and I don't think that's really what this library is for. As I understand it, the main purpose for this library is for use with lambdabot, to facilitate quick-n-dirty numeric stuff on top of the functionality Prelude provides. It may be true that "working around the flaws in the stand number prelude in a localized way just won't buy you that much," but that's pretty much what this library does, and I'm not inclined to radically alter it.

cartazio commented 10 years ago

@DanBurton agreed :)

DanBurton commented 10 years ago

@pthariensflame upon reflection, I don't like adding a dependency and pulling in the PartialOrdering class. It's obscure, and I don't think anybody will use it. (Do you have a use case for using this instance?)

So I'm inclined to mostly reject this pull request. There are, however, a couple things I'd like to bring in:

I like adding encodeFloat and decodeFloat for Dif. I think we should instead implement decodeFloat in a way consistent with the rest of the code:

decodeFloat = decodeFloat . val

It gives an answer of questionable meaningfulness for derivitives, but this is no different from a lot of those other instance methods.

I like the show/read changes.

I like ispan, but not ival'.

I like the Eq change, I think interval identity should be defined as equality of the endpoints of the interval, and anything else should be "not equal".

I don't like the current Ord instance, but I think it should stay. It just needs to be documented.

pthariensflame commented 10 years ago

@DanBurton Alright; feel free to just copy what you want from this and leave the reas. :)

DanBurton commented 10 years ago

I'm transferring ownership of the repository to to @jwiegley, so I'll just leave this PR open and let him decide how to handle it.

jwiegley commented 7 years ago

I've been too far removed from this project for too long, and I'm ready to give this up to the next maintainer, who can make a decision on this issue. @cartazio Are you up for that? Or @DanBurton do you want to take it back? I didn't end up using it as much as I had intended, and now my focus is elsewhere, and not on Haskell numerics.

cartazio commented 7 years ago

Happy to help. I've had some slow motion collabs in the math spheres. I will admit I've not been in the loop on the numbers package activity lately but can start taking an active poke at it mid / late Jan or early feb. have enough on the queue for next few weeks

On Fri, Jan 6, 2017 at 2:35 AM John Wiegley notifications@github.com wrote:

I've been too far removed from this project for too long, and I'm ready to give this up to the next maintainer, who can make a decision on this issue. @cartazio https://github.com/cartazio Are you up for that? Or @DanBurton https://github.com/DanBurton do you want to take it back? I didn't end up using it as much as I had intended, and now my focus is elsewhere, and not on Haskell numerics.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jwiegley/numbers/pull/10#issuecomment-270848643, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQwtGgRzOPN9-9c8Zk9ZslcOjIUwvgks5rPe7bgaJpZM4BdYVK .

jwiegley commented 2 years ago

Is there anyone who would like to maintain this package, and to have this GitHub repository transferred over to them?