Legalize scanl1/scanr1 over an empty vector #382

gksato · 2021-04-24T01:24:14Z

Now scanl1, scanl1', scanr1, scanr1' gives an empty vector for an
empty input.
The definitions of these functions working on vectors,
bundles and streams, the documentations attached to these
definitions, and the related tests are correspondingly modified.

In the former definition, the functions scanl1 and scanr1 refused to
process empty vector and simply errorred.
However these functions could, and now they do, simply return an empty
vector in case of the absence of elements, as the corresponding
functions in Data.List in base do.
With this modification,
postscanl' (<>) mempty
can be rewritten to more generally applicable
scanl1' (<>).

gksato · 2021-04-24T02:08:22Z

I'm looking through the failed checks, and the result looks so cryptic to me. macos-latest/8.6.5 tests fails in its doctest here, as does in my MacBook w/ ghc 8.6.5.
However the same test fails for current master(6b024a0) on my macBook with the same error message, but it seems like it doesn't in here...

lehins · 2021-04-24T02:15:02Z

macOS+ghc-8.6 error:

vector-doctest: user error (Language.Haskell.GhciWrapper.close: Interpreter exited with an error (ExitFailure (-10)))

I've seen this error many times in my other project and a few times in vector now. It looks like a bug either in doctests or in ghc and I have a gut feeling it has something to do with mutable boxed vectors used in doctests. In other words we need to investigate this issue, but it is unlikely that this PR is at fault here.

ghc-9.0+Windows error:

cabal.exe: Failed to build test:vector-tests-O0 from vector-0.13.0.1. The
failure occurred during the configure step.

The other problem with ghc-9 is a build error that comes from cabal and I am not sure what's at fault here

Shimuuar · 2021-04-24T12:56:19Z

Yes defining scanl1 f [] = [] makes sense.

As for doctests. They are very flaky. I'll try to figure out why they're failing

gksato · 2021-04-28T16:47:40Z

Thank you for your comments. I'm skimming through the logs of the checks, which all failed, from the second run triggered by my mistaken closure and reopen of this PR. I'm not at all familiar with GitHub CI and I've never read a single line of its document, but it looks as if make sdists had failed because the *.cabal files, or maybe whole package, had been absent due to the lack of the run of actions/checkout@v2. Maybe the following line is at blame:

vector/.github/workflows/ci.yml

Line 48 in 6b024a0

    
           if: github.event.action == 'opened' || github.event.action == 'synchronize' || github.event.ref == 'refs/heads/master'

Does it need to be fixed or not? Do we need a separate issue? I mean, re-testing is just a waste of computing resource if no new commit is added, but it is confusing the check mark on a PR is always replaced with a cross mark when its author accidentally closes and reopens it.

lehins · 2021-04-28T22:30:06Z

@Bodigrim Do you know why this line ^ is there? It doesn't make sense to me to not checkout code on CI

Shimuuar · 2021-04-29T07:23:19Z

I think it was copy-pasted from somewhere without understanding what does it do

Shimuuar · 2021-05-01T14:55:47Z

@gksato could you please rebase PR on top of current master and force push. That should trigger CI rebuild which will hopefully work

gksato · 2021-05-01T16:09:22Z

@Shimuuar Thank you for your work. I think I've done it. Let's see what happens...

gksato · 2021-05-01T16:17:29Z

Hooray, it worked!

Shimuuar · 2021-05-01T17:23:26Z

Could you add changelog entry? And note that prior to 0.13 calling them on empty vector resulted in error.

P.S. I wrote tool for copying documentation from Generic module: https://github.com/Shimuuar/vector-doctweak It doesn't always work but saves a lot of work

P.P.S. If you add doctest examples it would be absolutely great

gksato · 2021-05-02T00:15:15Z

Alrighty, I'll get it done today

Added a changelog entry for the legalization of scanl1/scanr1 [] (which is introduced in haskell#382) as one of 0.13 changes.

gksato · 2021-05-06T10:37:26Z

Yuck. I'm seeing some CI failure again. I've done it, anyway.

Shimuuar · 2021-05-23T07:54:47Z

Sorry for the delay. I've been terribly busy as of late.

For the record CI failures are:

ubuntu GHC7.10

Weird linking errors:

Linking /home/runner/work/vector/vector/dist-newstyle/build/x86_64-linux/ghc-7.10.3/vector-0.13.0.1/t/vector-tests-O0/build/vector-tests-O0/vector-tests-O0 ...
/usr/bin/ld: /home/runner/work/vector/vector/dist-newstyle/build/x86_64-linux/ghc-7.10.3/vector-0.13.0.1/t/vector-tests-O0/build/vector-tests-O0/vector-tests-O0-tmp/Main.o: relocation R_X86_64_32S against symbol `stg_upd_frame_info' can not be used when making a PIE object; recompile with -fPIE

And it's over 9 thousand lines about relocation R_X86_64_32S

Ubuntu GHC 8.6.5

Doctests fail with:

vector-doctest: /opt/ghc/8.6.5/bin/ghc-8.6.5: getPermissions:getFileStatus: does not exist (No such file or directory)

Ubuntu GHC 8.6.5

Same here

vector-doctest: /opt/ghc/8.8.4/bin/ghc-8.8.4: getPermissions:getFileStatus: does not exist (No such file or directory)

Shimuuar · 2021-05-23T08:43:45Z

I've restarted CI build and now everything works except for GHC7.10 which fails with linker error. I'm not sure what causes this problem and what to do with it

Other that that PR is good to go.

gksato · 2021-05-23T13:51:06Z

Thanks for inspecting. It looks we have the same error in the current HEAD. The library itself builds, but the linker fails to link the test executables. How bothering.

Shimuuar · 2021-05-23T21:46:40Z

CI also stands for constant irritation. I think I've fixed it. Could you please rebase on top of master again. Hopefully it will work this time

Now scanl1, scanl1', scanr1, scanr1' gives an empty vector for an empty input. The definitions of these functions working on vectors, bundles and streams, the documentations attached to these definitions (including addition of doctests), and the related tests are correspondingly modified. In the former definition, the functions scanl1 and scanr1 refused to process empty vector and simply errorred. However these functions could, and now they do, simply return an empty vector in case of the absence of elements, as the corresponding functions in Data.List in base do. With this modification, @ postscanl' (<>) mempty @ can be rewritten to more generally applicable @ scanl1' (<>) @.

Added a changelog entry for the legalization of scanl1/scanr1 [] (which is introduced in haskell#382) as one of 0.13 changes.

gksato · 2021-05-23T23:48:10Z

Thanks for your work. It passed.

Shimuuar · 2021-05-24T10:25:39Z

Finally! Thank you for your work and patience

# Changes in version 0.13.0.0 * `mkType` from `Data.Vector.Generic` is deprecated in favor of `Data.Data.mkNoRepType` * The role signatures on several `Vector` types were too permissive, so they have been tightened up: * The role signature for `Data.Vector.Mutable.MVector` is now `type role MVector nominal representational` (previously, both arguments were `phantom`). [#224](haskell/vector#224) * The role signature for `Data.Vector.Primitive.Vector` is now `type role Vector nominal` (previously, it was `phantom`). The role signature for `Data.Vector.Primitive.Mutable.MVector` is now `type role MVector nominal nominal` (previously, both arguments were `phantom`). [#316](haskell/vector#316) * The role signature for `Data.Vector.Storable.Vector` is now `type role Vector nominal` (previous, it was `phantom`), and the signature for `Data.Vector.Storable.Mutable.MVector` is now `type role MVector nominal nominal` (previous, both arguments were `phantom`). [#235](haskell/vector#235) We pick `nominal` for the role of the last argument instead of `representational` since the internal structure of a `Storable` vector is determined by the `Storable` instance of the element type, and it is not guaranteed that the `Storable` instances between two representationally equal types will preserve this internal structure. One consequence of this choice is that it is no longer possible to `coerce` between `Storable.Vector a` and `Storable.Vector b` if `a` and `b` are nominally distinct but representationally equal types. We now provide `unsafeCoerce{M}Vector` and `unsafeCast` functions to allow this (the onus is on the user to ensure that no `Storable` invariants are broken when using these functions). * Methods of type classes `Data.Vector.Generic.Mutable.MVector` and `Data.Vector.Generic.Vector` use concrete monads (`ST`, etc) istead of being polymorphic (`PrimMonad`, etc). [#335](haskell/vector#335). This makes it possible to derive `Unbox` with: * `GeneralizedNewtypeDeriving` * via `UnboxViaPrim` and `Prim` instance * via `As` and `IsoUnbox` instance: [#378](haskell/vector#378) * Add `MonadFix` instance for boxed vectors: [#312](haskell/vector#312) * Re-export `PrimMonad` and `RealWorld` from mutable vectors: [#320](haskell/vector#320) * Add `maximumOn` and `minimumOn`: [#356](haskell/vector#356) * The functions `scanl1`, `scanl1'`, `scanr1`, and `scanr1'` for immutable vectors are now defined when given empty vectors as arguments, in which case they return empty vectors. This new behavior is consistent with the one of the corresponding functions in `Data.List`. Prior to this change, applying an empty vector to any of those functions resulted in an error. This change was introduced in: [#382](haskell/vector#382) * Change allocation strategy for `unfoldrN`: [#387](haskell/vector#387) * Remove `CPP` driven error reporting in favor of `HasCallStack`: [#397](haskell/vector#397) * Remove redundant `Storable` constraints on to/from `ForeignPtr` conversions: [#394](haskell/vector#394) * Add `unsafeCast` to `Primitive` vectors: [#401](haskell/vector#401) * Make `(!?)` operator strict: [#402](haskell/vector#402) * Add `readMaybe`: [#425](haskell/vector#425) * Add `groupBy` and `group` for `Data.Vector.Generic` and the specialized version in `Data.Vector`, `Data.Vector.Unboxed`, `Data.Vector.Storable` and `Data.Vector.Primitive`. [#427](haskell/vector#427) * Add `toArraySlice` and `unsafeFromArraySlice` functions for conversion to and from the underlying boxed `Array`: [#434](haskell/vector#434)

gksato closed this Apr 24, 2021

gksato reopened this Apr 24, 2021

lehins mentioned this pull request Apr 28, 2021

Boxed mutable vector is buggy in doctests #385

Closed

gksato force-pushed the legalize-scan1-over-empty-vector branch from 0a9f106 to 660a181 Compare May 1, 2021 16:05

gksato force-pushed the legalize-scan1-over-empty-vector branch from 660a181 to a2b3ebc Compare May 6, 2021 10:27

gksato added a commit to gksato/vector that referenced this pull request May 6, 2021

Edit changelog for scan1 [] (haskell#382)

f2f9bd6

Added a changelog entry for the legalization of scanl1/scanr1 [] (which is introduced in haskell#382) as one of 0.13 changes.

gksato added 2 commits May 24, 2021 08:34

Edit changelog for scan1 [] (haskell#382)

5da3d25

Added a changelog entry for the legalization of scanl1/scanr1 [] (which is introduced in haskell#382) as one of 0.13 changes.

gksato force-pushed the legalize-scan1-over-empty-vector branch from f2f9bd6 to 5da3d25 Compare May 23, 2021 23:35

Shimuuar merged commit 3bc78cb into haskell:master May 24, 2021

gksato mentioned this pull request Jul 27, 2021

scanr1 and scanl1 on empty vector #395

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Legalize scanl1/scanr1 over an empty vector #382

Legalize scanl1/scanr1 over an empty vector #382

gksato commented Apr 24, 2021

gksato commented Apr 24, 2021

lehins commented Apr 24, 2021

Shimuuar commented Apr 24, 2021

gksato commented Apr 28, 2021 •

edited

Loading

lehins commented Apr 28, 2021

Shimuuar commented Apr 29, 2021

Shimuuar commented May 1, 2021

gksato commented May 1, 2021

gksato commented May 1, 2021

Shimuuar commented May 1, 2021

gksato commented May 2, 2021

gksato commented May 6, 2021

Shimuuar commented May 23, 2021

Shimuuar commented May 23, 2021

gksato commented May 23, 2021

Shimuuar commented May 23, 2021

gksato commented May 23, 2021

Shimuuar commented May 24, 2021

Legalize scanl1/scanr1 over an empty vector #382

Legalize scanl1/scanr1 over an empty vector #382

Conversation

gksato commented Apr 24, 2021

gksato commented Apr 24, 2021

lehins commented Apr 24, 2021

Shimuuar commented Apr 24, 2021

gksato commented Apr 28, 2021 • edited Loading

lehins commented Apr 28, 2021

Shimuuar commented Apr 29, 2021

Shimuuar commented May 1, 2021

gksato commented May 1, 2021

gksato commented May 1, 2021

Shimuuar commented May 1, 2021

gksato commented May 2, 2021

gksato commented May 6, 2021

Shimuuar commented May 23, 2021

ubuntu GHC7.10

Ubuntu GHC 8.6.5

Ubuntu GHC 8.6.5

Shimuuar commented May 23, 2021

gksato commented May 23, 2021

Shimuuar commented May 23, 2021

gksato commented May 23, 2021

Shimuuar commented May 24, 2021

gksato commented Apr 28, 2021 •

edited

Loading