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

Upgrade to GHC 9.10, bump nixpkgs #159

Merged
merged 45 commits into from
Feb 12, 2025
Merged

Upgrade to GHC 9.10, bump nixpkgs #159

merged 45 commits into from
Feb 12, 2025

Conversation

ngua
Copy link
Contributor

@ngua ngua commented Feb 12, 2025

  • Updates haskell-nix input to recent commit
  • Upgrades nixpkgs accordingly
  • Changes compiler version to GHC 9.10
  • Changes formatter to Fourmolu (see notes below for more)
  • Upgrades NVIDIA drivers for V100 GPU (amazingly working somehow)
  • Fixes upgrade-related problems with VSCode packages for Inferno
  • Fixes some overlay and haskell.nix related configuration for newer version of Hasktorch

Everything is building fine, including our CPU/GPU Inferno ML images (tested elsewhere, running on actual EC2 instances). I didn't need to make any changes to the Haskell code for the upgrade, just configuration changes.

NOTE: There are some compiler warnings now that are present on GHC 9.10 but not on earlier versions. I'd prefer not to make a CPP mess of a large number of Haskell modules, so I've left those unaddressed. Once we switch to GHC 9.10+ everywhere, we can fix them.

# Using `hackage-package` will prevent building `ormolu`
# from interfering with the build plan (incl. incompatible
# compiler versions)
o = pkgs.haskell-nix.hackage-package {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've switched the formatter to Fourmolu, for several reasons:

  • We use it elsewhere
  • It's better than Ormolu
  • Even staying with Ormolu will require a large reformatting of the Haskell codebase, so we might as well switch to Fourmolu

I've disabled the CI check for formatting for now. Once the upgrade is in and confirmed working everywhere, I will re-enable it in a re-formatting PR

author: Sam Balco
maintainer: [email protected]
build-type: Simple
cabal-version: 2.4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cabal-fmt'd the Cabal files, the inconsistency was bugging me. I can add a formatting check for this in the future re-formatting PR

, bytestring >= 0.10.10 && < 0.12
, containers >= 0.6.2 && < 0.7
, cryptonite >= 0.30 && < 0.31
, base
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to get rid of the strict version bounds, again for several reasons:

  • We use Nix for this and don't need to involve the Cabal bounds solver again
  • This is our practice elsewhere
  • We added these version bounds only to appease cabal sdist for release to Hackage, but we haven't done a Hackage release in ~2 years. I don't think we even should, since a lot of the project would be very difficult to build outside of Nix
  • Having these constraints makes upgrading harder
  • cabal gen-bounds doesn't help very much

Left err -> print err
Right ast ->
evalExpr defaultEnv Map.empty ast >>= \case
getArgs >>= \case
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this executable fairly early on so changed it to avoid head (which emits a warning on GHC 9.10). However, there are unfortunately several other places in the codebase where head is used. I've decided to leave that for another day, as it involves a fairly large refactor (either that or disabling the partial warning, which I think is a good thing and should be left enabled)

, config
, ghcOptions ? [ ]
, profiling ? false
# Must be of the form: { device = <cpu|cuda-10|cuda-11>; }
# Must be of the form: { device = <cpu|cuda-118>; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can only build with CUDA 11.8 now

@@ -1,17 +1,17 @@
final: prev: {
# Override the default NVIDIA driver with one known to work well with
# CUDA
# the V100 GPU
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been tested and is working on a V100 GPU

inherit device cudaSupport;
};

(prev.libtorch-bin.override { inherit cudaSupport; }).overrideAttrs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hasktorch is on a slightly older version of libtorch-bin and Pytorch (2.3.0), hence the overrides

@@ -27,45 +28,46 @@
};
};

nonReinstallablePkgs = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC I originally added these only so that Hasktorch would build, back when we first began the Hasktorch integration. However, it seems that it no longer builds with nonReinstallablePkgs specified. Removing it entirely works

@ngua
Copy link
Contributor Author

ngua commented Feb 12, 2025

Just need to fix the pytorch shell for CI, something is wrong with the triton package

@ngua ngua merged commit c8ee235 into main Feb 12, 2025
1 check passed
@ngua ngua deleted the rory-ghc-nixpkgs-upgrade branch February 12, 2025 10:45
@ngua ngua restored the rory-ghc-nixpkgs-upgrade branch February 17, 2025 07:04
ngua added a commit that referenced this pull request Feb 19, 2025
Bumps haskell.nix flake input to a slightly newer commit than in #159 so
that we can use 24.11 as a new stable branch. Also adds a few small
changes (adding `hlint` to the shell env, etc...).

The new nixpkgs for building images is working (I've confirmed
elsewhere).

No Haskell changes.
ngua added a commit that referenced this pull request Feb 21, 2025
Only a formatting PR, follow-up to #159. No other changes were
introduced.
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