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

WIP: Migration to vector #147

Closed
wants to merge 13 commits into from

Conversation

tau3
Copy link
Contributor

@tau3 tau3 commented Oct 18, 2018

Hello!

Here is the initial implementation of migration from arrays to vectors (#79) It compiles but fails against some of unit tests. To figure out the root cause, i recompiled sources with enabled check-bounds Yep, seems like most of exceptions were 'index of bounds'. I wanted to debug in cabal-repl, but it failed to load due to Error: bytecode compiler can't handle unboxed tuples and sums. Cabal suggested to specify -fobject-code, but i couldn't google anything about that flag.

So, some assistance is required :)

@tau3
Copy link
Contributor Author

tau3 commented Oct 18, 2018

P.S. diff shows realy huuuuge changeset because of hindent (default setup). If a special profile is used in Arithmoi, please, point me at so i can reduce the number of changed lines.

@rockbmb
Copy link
Contributor

rockbmb commented Oct 18, 2018

@tau3 when you run cabal repl or cabal new-repl, simply pass -fobject-code as a GHC option:

cabal (new-)repl --ghc-options=-fobject-code

This goes for any other GHC option(s) you may wish to pass to a GHCi session.

@Bodigrim
Copy link
Owner

Cabal suggested to specify -fobject-code, but i couldn't google anything about that flag.

https://downloads.haskell.org/~ghc/8.4.1/docs/html/users_guide/ghci.html#compiling-to-object-code-inside-ghci

P.S. diff shows realy huuuuge changeset because of hindent (default setup). If a special profile is used in Arithmoi, please, point me at so i can reduce the number of changed lines.

I do not use hindent for artithmoi, so no agreed profile exists. We can discuss indentation profile as a separate issue, but in course of this PR I am not keen to review thousand lines. Please commit only non-whitespace changes. You can do it as suggested at https://stackoverflow.com/a/7149602 plus several manual tweaks.

@tau3
Copy link
Contributor Author

tau3 commented Oct 19, 2018

@Bodigrim thanks for advices, but unfortunatelly git diff ... -U0 -w ... didn't do the trick with spaces in this case 😞

also i encountered some weird vector bounds errors during debug (debugging itself was kinda stressing due to inability to work with unboxed code in interpreter mode).

probably this refactoring attempt should be considered as unseccessful.

@rockbmb thank you and good luck with yours!)

@rockbmb
Copy link
Contributor

rockbmb commented Oct 19, 2018

@tau3 don't call it unsuccessful just yet! You don't have to do everything at once.

You can just pick a module that is all four of:

  • small,
  • simple
  • few other modules depend on it
  • is still using Arrays instead of Vectors

and make the necessary refactoring. It won't be too difficult, and remember that after it compiles you have a friend in cabal (new-)test.

@Bodigrim
Copy link
Owner

+1 to @rockbmb
It may be tough to migrate prime sieves from array to vector, but migration of other modules is also highly appreciated.

@tau3
Copy link
Contributor Author

tau3 commented Oct 23, 2018

well, there is an option to contain methods for both arrays and vectors in Unsafe module, so the issue could be solved step-by-step.

@Bodigrim
Copy link
Owner

Bodigrim commented Jan 5, 2019

Closing due to the lack of activity. Anyway thanks for you efforts!

@Bodigrim Bodigrim closed this Jan 5, 2019
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.

3 participants