Skip to content
This repository was archived by the owner on Sep 20, 2023. It is now read-only.

Conversation

adinapoli
Copy link
Contributor

@adinapoli adinapoli commented Sep 3, 2016

This is a wip implementation of ChunkedUArray. Please chime in if you think what I have got so far looks fishy.

Feature list:

  • Datatype definition
  • Monoid instance functions
  • IsList instance functions
  • Sequential instance functions
  • IndexedCollection instance functions
  • Collection instance functions
    • take
    • cons
    • find
    • drop
    • snoc
    • sortBy (mocked)
    • ...
  • Core API functions
    • index
    • unsafeIndex
    • read
    • write
    • ...
  • Tests (I backed off as i wanted to wait for the testsuite overhaul).

@adinapoli adinapoli added the WIP Work In Progress (e.g. don't merge) label Sep 3, 2016
@JanGe JanGe added C - design design stuff C - collection collection stuff labels Sep 3, 2016
-- |
-- Module : Foundation.Array.ArrayUArray
-- License : BSD-style
-- Maintainer : Vincent Hanquez <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

Probably should be you, or "Foundation" :)

@vincenthz
Copy link
Member

so far it look fine ! One thing that might be a good idea would be to find a better name for this structure; while ArrayUArray describe the structure quite well, it's not a very nice looking name. if anyone have any suggestion to what we could call it :)

@JanGe
Copy link
Member

JanGe commented Sep 5, 2016

ChunkedArray? UChunks? UBuffers? (although they all sound weird)

@adinapoli
Copy link
Contributor Author

ChunkedArray is not bad, and I would prefer an "opaque" name rather than using something with the U in it. What about MultiArray?

@NicolasDP
Copy link
Member

MultiArray sounds good

@JanGe
Copy link
Member

JanGe commented Sep 5, 2016

2DArray? From a MultiArray I would expect support for more than two dimensions.

@vincenthz
Copy link
Member

vincenthz commented Sep 5, 2016

I have to agree with @JanGe. MultiArray makes me think of something with dimensions. There's also a good side too so I guess I could warm up to it

@adinapoli
Copy link
Contributor Author

adinapoli commented Sep 6, 2016

Good point guys! I don't mind 2DArray, so I guess we could simply go for it and see "how it feels". At the end of the day it's something easy enough to be changed later with relatively no pain 😉

Something we still haven't figured out though is how to handle the "boxed" variant. So far 2DArray is really a "boxed outer array with an unboxed content", whereas we said it would be useful to have something like Array (Array ty), so looks like the name question is gonna croup up again 😁

@ndmitchell
Copy link
Contributor

I would go for ChunkedUArray and ChunkedArray. The semantics is that data is stored in chunks. Anything mentioning array twice, or 2, or multi, conjures up the wrong idea.

@adinapoli adinapoli changed the title [WIP] Issue #47 (ArrayUArray) [WIP] Issue #47 (ChunkedUArray) Sep 10, 2016
@adinapoli
Copy link
Contributor Author

I'm slowly making progress on this! I have fixed the implementation for take and will try to fix drop early this week 😉

@adinapoli
Copy link
Contributor Author

I've added the implementation for find and cons.

Only 200 failing tests to go 😂 😂

@vincenthz
Copy link
Member

I'ld recommend just doing the basic, and have everything else using x = fromList . x . toList. it's easier to have everything in place this way (tests, benchs), and it's way less daunting this way. You're making good progress regardless !

@adinapoli
Copy link
Contributor Author

I'ld recommend just doing the basic, and have everything else using x = fromList . x . toList. it's easier to have everything in place this way (tests, benchs), and it's way less daunting this way.

That's actually a good suggestion! I think once I implement drop the rest of the functions are not too daunting if not for the sorting ones, where sorting needs "compaction" anyway, so I think I will stick with the naive fromList . x . toList suggestion for those -- thanks Vincent! 😉

@adinapoli
Copy link
Contributor Author

I'm definitely still on this (just working on it very sporadically due to life getting in the way). Once I get a correct drop implementation I'm gonna mock out the rest of the Sequential typeclass with the fromList . toList thing so we can at least start getting green builds!

@adinapoli
Copy link
Contributor Author

Hey @vincenthz ! I have just completed the first pass of the ChunkedUArray data structure, where only a tiny subsets of operations are implemented "natively" (e.g. take, drop, etc), all the others use the fromList . ops . toList trick.

I'm puzzled about why Travis is failing on some builds, do you have any idea here?

Thank you guys!

@JanGe
Copy link
Member

JanGe commented Oct 12, 2016

That looks like the issue we fixed in #127 and should go away when you rebase your branch on master.

@adinapoli
Copy link
Contributor Author

Hey @JanGe ! Thanks for chiming in. I already merged upstream/master and tried a new build in my latest commit, but unfortunately Travis keeps failing.

The weird thing is that is failing only for certain environments (and thus not consistently) with vague errors or StackOverflow ones. Unless I did something really silly in my "native" implementations (the one not using the from/toList trick), I'm wondering if what's causing trouble to Travis is simply the sheer amount of tests it needs to run.

How do you guys think is the best approach to troubleshoot this? Use @ndmitchell 's technique to identify space leaks trying to pinpoint if the cause is my code or if is lurking elsewhere?

@vincenthz
Copy link
Member

vincenthz commented Oct 13, 2016

The stack overflow is due to old ghc that doesn't have support the MINIMUM pragma in typeclass, and not implementing the new methods elem, notElem (which happen to recursively depend on each other, busy looping)

@adinapoli
Copy link
Contributor Author

adinapoli commented Oct 13, 2016

not implementing the new methods elem, notElem

Ouch, that's embarrassing, @vincenthz ! I totally missed the extra methods for Collection, my bad! I came up with some interim implementations, just for the sake of making Travis happy.

Looks like we have a green build now, many thanks! (at the moment of writing Travis successfully built all the mandatory envs and is completely the last "failable" one). What do you reckon should be the next steps in order to make progress on this? At which point do you feel we should merge this and spin up ancillary tickets to improve the implementation of the stubbed methods?

@vincenthz
Copy link
Member

No worries about the collection new methods, there were added whilst you were adding this, so this is somewhat expected.

yes, I think we shouldn't wait for merging this. It's an awesome step already, and well done for completing the initial phase ! We can now incrementally add and improve things.

@adinapoli
Copy link
Contributor Author

yes, I think we shouldn't wait for merging this. It's an awesome step already, and well done for completing the initial phase !

Thanks man! Feel free to merge whenever that makes sense. 🎉

@vincenthz vincenthz changed the title [WIP] Issue #47 (ChunkedUArray) Add ChunkedUArray Oct 14, 2016
@vincenthz vincenthz removed the WIP Work In Progress (e.g. don't merge) label Oct 14, 2016
@vincenthz vincenthz merged commit 47e34cd into haskell-foundation:master Oct 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

C - collection collection stuff C - design design stuff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants