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

Bump Nix/CI, drop support for GHC 8.10, and add support for GHC 9.4 and 9.6 #422

Merged
merged 19 commits into from
Feb 5, 2024

Conversation

ConnorBaker
Copy link
Contributor

@ConnorBaker ConnorBaker commented Nov 29, 2023

Updates the Nixpkgs pin to a more recent copy of master.

Additionally, updates the CI to:

  • Bump the versions of the actions used
  • Move away from deprecated GitHub runner commands and towards environment files
  • Attempt to cache the nix portion of the linux/macOS workflows
  • Use shorter and more descriptive cache keys (thank you, Bumps for GHC 9.8, fix building with unix-2.8, update CI #420 for suggesting the change)
  • Add different shells for different LTS releases so the version of GHC provisioned matches that of the LTS (at least in the minor version -- it may differ in the patch version)
  • Add the ability to override the version of R used
  • Drop support for GHC 8.10
  • Add support for GHC 9.4 and 9.6.

This should unblock the work #420 started.

@ConnorBaker ConnorBaker self-assigned this Nov 29, 2023
@ConnorBaker
Copy link
Contributor Author

Currently blocked on errors like Stack build plans failing due to missing cryptographic hashes (https://github.com/tweag/HaskellR/actions/runs/7036651826/job/19149756100#step:7:2884)

Error: [S-7282]
       Stack failed to execute the build plan.
       
       While executing the build plan, Stack encountered the error:
       
       Error: [S-922]
       No cryptographic hash found for Hackage package splitmix-0.1.0.4
Error: Process completed with exit code 1.

I suspect this might have to do with the way caching is set up in this PR; perhaps something is being clobbered or we must separate the cache for each version of GHC we support?

@ConnorBaker
Copy link
Contributor Author

Another error, this time due to a test suite failure with inline-r (specifically test-qq) on GHC 8.10.7 with the Stack 18 LTS. (https://github.com/tweag/HaskellR/actions/runs/7036892643/job/19150504895#step:8:5552).

Error: [S-7282]
       Stack failed to execute the build plan.
       
       While executing the build plan, Stack encountered the error:
       
       Error: [S-1995]
       Test suite failure for package inline-r-1.0.1
           test-qq:  exited with: ExitFailure (-11)
           tests:  exited with: ExitFailure (-11)
       Logs printed to console
       
Error: Process completed with exit code 1.

I'm able to reproduce locally via:

nix-shell
mv stack.yaml stack-latest.yaml
mv stack.yaml.lock stack-latest.yaml.lock
mv stack-lts-18.yaml stack.yaml
stack --nix build
stack --nix test

@ConnorBaker
Copy link
Contributor Author

I think the failing tests all have to do with Haskell/R function interop.

The Haskell Function from R unit test fails:

, testCase "Haskell function from R" $ do
(((3::Double) @=?) =<<) $ fmap fromSEXP $
alloca $ \p -> do
e <- peek R.globalEnv
R.withProtected (mkSEXPIO $ \x -> return $ x + 1 :: R s Double) $
\sf -> R.withProtected (mkSEXPIO (2::Double)) $ \d ->
R.withProtected (R.lang2 sf d) (unsafeRunRegion . eval)
>>= \(R.SomeSEXP s) ->
R.cast (sing :: R.SSEXPTYPE 'R.Real) <$>
R.tryEval s (R.release e) p

And the failing tests in test-qq (https://github.com/tweag/HaskellR/blob/63afa4bc78d90f93630127003e25b0f53fb0815a/inline-r/tests/test-qq.hs) which I've commented out below all deal with the same.

@facundominguez any ideas?

-- |
-- Copyright: (C) 2013 Amgen, Inc.
--
-- Run H on a number of R programs of increasing size and complexity, comparing
-- the output of H with the output of R.

{-# LANGUAGE DataKinds #-}
{-# LANGUAGE QuasiQuotes #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE ViewPatterns #-}
{-# LANGUAGE OverloadedStrings #-}

module Main where

import qualified Foreign.R as R
import H.Prelude as H
import qualified Data.Vector.SEXP as SVector
import qualified Data.Vector.SEXP.Mutable as SMVector
import Control.Memory.Region
import Data.Text (Text)

import Control.Applicative
import Control.Monad.Trans (liftIO)
import Data.Int
import Data.Singletons (sing)
import Test.Tasty.HUnit hiding ((@=?))
import Prelude -- Silence AMP warning

hFib :: SEXP s 'R.Int -> R s (SEXP s 'R.Int)
hFib n@(H.fromSEXP -> 0 :: Int32) = fmap (flip R.asTypeOf n) [r| 0L |]
hFib n@(H.fromSEXP -> 1 :: Int32) = fmap (flip R.asTypeOf n) [r| 1L |]
hFib n = (`R.asTypeOf` n) <$> [r| hFib_hs(n_hs - 1L) + hFib_hs(n_hs - 2L) |]

-- | Version of '(@=?)' that works in the R monad.
(@=?) :: Literal a b => String -> a -> R s ()
expected @=? actual = liftIO $ do
    let actualstr = cast SString [rsafe| deparse(actual_hs) |]
    assertEqual "" expected (fromSEXP actualstr)

main :: IO ()
main = H.withEmbeddedR H.defaultConfig $ H.runRegion $ do

    -- FAILING TEST
    -- Placing it before enabling gctorture2 for speed.
    -- ("4181L" @=?) =<< hFib =<< H.mkSEXP (19 :: Int32)

    _ <- [r| gctorture2(1,0,TRUE) |]

    ("1" @=?) =<< [r| 1 |]

    ("1" @=?) =<< return [rsafe| 1 |]

    ("3" @=?) =<< [r| 1 + 2 |]

    ("3" @=?) =<< return [rsafe| base::`+`(1, 2) |]

    ("c(\"1\", \"2\", \"3\")" @=?) =<< [r| c(1,2,"3") |]

    ("2" @=?) =<< [r| x <<- 2 |]

    ("3" @=?) =<< [r| x+1 |]

    let y = (5::Double)
    ("6" @=?) =<< [r| y_hs + 1 |]

    _ <- [r| z <<- function(y) y_hs + y |]
    ("8" @=?) =<< [r| z(3) |]

    ("1:10" @=?) =<< [r| y <<- c(1:10) |]

    -- FAILING TESTS
    -- let foo1 = (\x -> (return $ x+1 :: R s Double))
    -- let foo2 = (\x -> (return $ map (+1) x :: R s [Int32]))
    -- ("3" @=?) =<< [r| mapply(foo1_hs, 2) |]
    -- ("2:11" @=?) =<< [r| mapply(foo2_hs, y) |]
    -- ("8" @=?) =<< [r| foo1_hs(7) |]

    ("43" @=?) =<< [r| x <<- 42 ; x + 1 |]

    let xs = [1,2,3]::[Double]
    ("c(1, 2, 3)" @=?) =<< [r| xs_hs |]


    ("NULL" @=?) H.nilValue

    -- FAILING TEST
    -- let foo3 = (\n -> fmap fromSomeSEXP [r| n_hs |]) :: Int32 -> R s Int32
    -- ("3L" @=?) =<< [r| foo3_hs(3L) |]

    -- FAILING TEST
    -- let foo4 = (\n m -> return $ n + m) :: Double -> Double -> R s Double
    -- ("99" @=?) =<< [r| foo4_hs(33, 66) |]

    -- FAILING TEST
    -- let fact (0 :: Int32) = return 1 :: R s Int32
    --     fact n = fmap dynSEXP [r| n_hs * fact_hs(n_hs - 1L) |]
    -- ("120L" @=?) =<< [r| fact_hs(5L) |]

    -- FAILING TEST
    -- let foo5  = \(n :: Int32) -> return (n+1) :: R s Int32
    -- let apply :: R.SEXP s 'R.Closure -> Int32 -> R s (R.SomeSEXP s)
    --     apply n m = [r| n_hs(m_hs) |]
    -- ("29L" @=?) =<< [r| apply_hs(foo5_hs, 28L ) |]

    -- test Vector literal instance
    v1 <- do
      x <- SMVector.new 3 :: R s (SMVector.MVector s 'R.Int Int32)
      SMVector.unsafeWrite x 0 1
      SMVector.unsafeWrite x 1 2
      SMVector.unsafeWrite x 2 3
      return x
    let v2 = SMVector.release v1 :: SMVector.MVector V 'R.Int Int32
    ("c(7, 2, 3)" @=?) =<< [r| v = v2_hs; v[1] <- 7; v |]
    io . assertEqual "" "fromList [1,2,3]" . Prelude.show =<< SVector.unsafeFreeze v1

    let utf8string = "abcd çéõßø" :: String
    io . assertEqual "" utf8string =<< fromSEXP <$> R.cast (sing :: R.SSEXPTYPE 'R.String) <$> [r| utf8string_hs |]

    let utf8string1 = "abcd çéõßø" :: Text
    io . assertEqual "" utf8string1 =<< fromSEXP <$> R.cast (sing :: R.SSEXPTYPE 'R.String) <$> [r| utf8string1_hs |]

    -- Disable gctorture, otherwise test takes too long to execute.
    _ <- [r| gctorture2(0) |]
    let x = ([1] :: [Double])
    ("3" @=?) =<< [r| v <- x_hs + 1
                      v <- v + 1
                      v |]

    return () 

@facundominguez
Copy link
Member

No deep insights. I'm wondering if the upgrade of nixpkgs is changing the R version in use.

@ConnorBaker
Copy link
Contributor Author

No deep insights. I'm wondering if the upgrade of nixpkgs is changing the R version in use.

It looks like it. From the previous pin:

$ nix eval nixpkgs/a115bb9bd56831941be3776c8a94005867f316a7#R.version
"4.2.1"

And from the new pin:

$ nix eval nixpkgs/28952798b6803c0f5e4d2b154cbdcb37c80dd15d#R.version
"4.3.2"

If I can find some time I'll try to see if overriding the version we use with the older release of R fixes the tests.

@ConnorBaker
Copy link
Contributor Author

@facundominguez adding an override for R seems to have done the trick; looks like 4.3.x include API changes from 4.2 which break HaskellR.

@ConnorBaker ConnorBaker marked this pull request as ready for review December 4, 2023 19:56
@facundominguez
Copy link
Member

Thanks @ConnorBaker! Looks like there are some segfaults to investigate still.

@ConnorBaker
Copy link
Contributor Author

@facundominguez I think this PR is ready to be merged; there's still a failure on Linux with LTS 18 (GHC 8.10.7), but that's unrelated to this PR and fixing that is out of the scope of this PR.

What are your thoughts?

Copy link
Member

@facundominguez facundominguez left a comment

Choose a reason for hiding this comment

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

LGTM

Do you have any insights about why GHC 8.10.7 doesn't work?

Should CI be configured to ignore or disable the lts-18 job?

.github/workflows/workflow.yaml Outdated Show resolved Hide resolved
@ConnorBaker
Copy link
Contributor Author

Do you have any insights about why GHC 8.10.7 doesn't work?

Unfortunately, no :(

I think it might have something to do with a GLIBC bump between the versions of the Nixpkgs, but I haven't been able to sit down with GDB to figure out what exactly is causing the issue.

Should CI be configured to ignore or disable the lts-18 job?

I've removed it from the CI, as well as the shell and lock file for that LTS; I don't think it's right to leave the stack file or shell when we no longer support that release.

@facundominguez with these latest changes, and I okay to merge (assuming CI is green)?

@ConnorBaker ConnorBaker changed the title Bump Nixpkgs pin and update CI Bump Nix/CI, drop support for GHC 8.10, and add support for GHC 9.4 and 9.6 Feb 5, 2024
@facundominguez
Copy link
Member

facundominguez with these latest changes, and I okay to merge (assuming CI is green)?

I think so :) 🎉

@ConnorBaker ConnorBaker enabled auto-merge February 5, 2024 15:39
@ConnorBaker ConnorBaker merged commit 472d4b4 into master Feb 5, 2024
10 checks passed
@ConnorBaker ConnorBaker deleted the feat/nix-bump-pin branch February 5, 2024 15:43
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.

2 participants