mikeizbicki / subhask

Type safe interface for working in subcategories of Hask
BSD 3-Clause "New" or "Revised" License
418 stars 43 forks source link

ghc 8 roadmap and cleanup #40

Open tonyday567 opened 8 years ago

tonyday567 commented 8 years ago

Hi Mike,

Do you have a sense of when, where and what will be the path towards the ghc 8 refactor and version?

In particular, I was thinking of doing some house-cleaning - commented-out code deletion, move towards a -Wall build, filling out of a bench, and general tightening up of the codebase without making any changes. Would this be useful, however, given the changes you have in mind?

mikeizbicki commented 8 years ago

Thanks for asking Tony. The short answer is yes, these changes would be useful.

The long answer is: all of my Haskell time for the past few months has been working on a submission to the Haskell Symposium. This submission is about a clever way to deal with floating point calculations. The deadline for the submission is tomorrow, so I won't be working on it any more after that. Over the next few weeks, my Haskell time will be spent incorporating these techniques into subhask and in the process updating it to GHC 8. Most of the code that will be affected by this is the testing code. I think the changes you're proposing could only make this process go smoother. I'm hoping the process doesn't take too long, but things always seem to take longer than I hope.

tonyday567 commented 8 years ago

45

The major holes are:

tonyday567 commented 8 years ago

Just looking at the current build warnings, and there's only a few categories:

  1. "Ignoring unusable UNPACK pragma on the first argument of"

This is a proper thing right? eg an !Int can't be unpacked cause it has more than one data constructor etc? So go ahead and delete these unpacks (keeping the strict annotation)?

  1. No explicit implementation

Add -fno-warn-missing-methods to the cabal file and work it out over time.

  1. In the use of type constructor or class ‘NoIO’ (imported from GHC.GHCi): "This is an unstable interface."

Looks very benign - turn warning off in affected files.

Once these are fixed up, it will be (almost) -Wall.

mikeizbicki commented 8 years ago

GHC can't unpack polymorphic types, but it can unpack any concrete type. The unpack pragmas warned about by ghc aren't doing anything and should be removed (keeping the strictness anotation).

The missing methods should get fixed when I port over the improved class hierarchy for ghc 8. Your temporary solution is fine.

I think that's generated due to some template haskell making tons of automatic instances. Turning off the warning should be fine. Relatedly, I think there's TH generating a bunch of debug output, and that should probably be cleaned up somehow. Ideally, there'd be some sort of debug flag we could pass to cabal to get the information optionally printed.

tonyday567 commented 8 years ago

Ok. Will get these done over the weekend.

I think there's a standard -fdebug in ghc and then you use cpp directives in the code.

On Fri, Jun 17, 2016 at 10:04 AM Mike Izbicki notifications@github.com wrote:

GHC can't unpack polymorphic types, but it can unpack any concrete type. The unpack pragmas warned about by ghc aren't doing anything and should be removed (keeping the strictness anotation).

The missing methods should get fixed when I port over the improved class hierarchy for ghc 8. Your temporary solution is fine.

I think that's generated due to some template haskell making tons of automatic instances. Turning off the warning should be fine. Relatedly, I think there's TH generating a bunch of debug output, and that should probably be cleaned up somehow. Ideally, there'd be some sort of debug flag we could pass to cabal to get the information optionally printed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mikeizbicki/subhask/issues/40#issuecomment-226648042, or mute the thread https://github.com/notifications/unsubscribe/ABjATR4zx2E5U5pTq6Doo5WbpbBZ6lCHks5qMeSegaJpZM4IyoQb .

tonyday567 commented 8 years ago

In fact, now I think about it, the 3 examples (4 once the matrix one gets cleaned up) can usefully become simple test suites, somewhat redundant but orthogonal to the main suite structure.

Same thoughts on bench - record performance as it stands for the main functionality in each example, across small and large sized things. I've tried mixing test, bench and example in the one file before, and it tends to get too busy in the code to be clear

mikeizbicki commented 8 years ago

Yeah, the idea of them being test suites was to ensure that they would always continue to compile after making changes to the library.

I'd love the ability to somehow automatically catch major regressions in benchmark times, but I have no idea how that would work.

tonyday567 commented 8 years ago

I've done a similar regression thing before. Save the performance results in a file, like a golden file test, and include a comparison in the bench. The main problem is the criterion api, which tends to spam stdout. You need to go lower level and grab exact results.

mikeizbicki commented 8 years ago

If you didn't notice, there's a ghc-8.0 branch now. It compiles and passes all tests, and has a minor cleaning up of the Eq hierarchy. Before merging with master, I want to:

That branch has the changes you've made for -Wall under 7.10, but there's a number of additional fixes needed for 8.0. A lot of these changes are closely tied to the new Eq hierarchy, so I'll probably have to make them. Hopefully, I'll get a chance to finish that this week.

mikeizbicki commented 8 years ago

Another problem with the golden file idea is that it requires the builds happen on the same machine; I don't think travis can guarantee this. A better (I think) solution would be to download the current commit and the previous commit, run them both, and pass if the ratio of the times is under some threshold. That sounds like a lot of work to make happen though.

tonyday567 commented 8 years ago

Just maybe, the rdtsc measure in criterion might do the trick - it measures cycles rather than time, and that should be stable across machines.

tonyday567 commented 8 years ago

And, no, I didn't notice 8.0 at all! Does it include the floating point stuff yet?

mikeizbicki commented 8 years ago

Not yet. That's sitting in a different repo and I'll merge the two once I've finished the two bullets above.