Skip to content

python3Packages.elegy: init at 0.8.4#154115

Merged
samuela merged 1 commit intoNixOS:masterfrom
ndl:submit/elegy
Jan 10, 2022
Merged

python3Packages.elegy: init at 0.8.4#154115
samuela merged 1 commit intoNixOS:masterfrom
ndl:submit/elegy

Conversation

@ndl
Copy link
Contributor

@ndl ndl commented Jan 9, 2022

Motivation for this change

Finishes the "extra JAX libraries / frameworks" implementation, see the discussion in #152754 I also had to adjust relax-deps.patch for treex as due to PyYAML version update in nixpkgs it was out of allowed range.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ndl ndl requested review from FRidh and jonringer as code owners January 9, 2022 09:08
@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Jan 9, 2022
@ndl ndl requested a review from samuela January 9, 2022 09:09
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 9, 2022
Comment on lines +64 to +65
Copy link
Member

Choose a reason for hiding this comment

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

@samuela
Copy link
Member

samuela commented Jan 9, 2022

It would be nice to track down the cause of the failing test, but I don't want that to be a blocker for merging this PR. So unless you'd like me to hold off @ndl, I'll merge in a day or two unless anyone else objects.

@ndl
Copy link
Contributor Author

ndl commented Jan 9, 2022

Yeah, agree it would be nice to track it down, but realistically I'm unlikely to look into this soon + it's not even clear if this would happen outside the tests, so I'd prefer to leave it "as is" for now.

@samuela
Copy link
Member

samuela commented Jan 10, 2022

Unfortunately I believe nixpkgs-review is stuck in some kind of live lock building one of the dependencies for this PR. It's been stuck on this step:

[1/19/23 built (7 failed), 381 copied (3912.8/3914.3 MiB), 1382.7 MiB DL] building python3.9-einops-0.3.2 (installCheckPhase): tests.test_layers.test_reduce_symbolic ... ok

for around half an hour now. About half of my CPUs have been pegged to 100% the whole time.

I know that this PR doesn't affect einops directly but we ought to get to the bottom of this nonetheless.

@samuela
Copy link
Member

samuela commented Jan 10, 2022

Created for the einops issue: #154251. Not sure that we need to worry about that for now though.

@ndl How did you manage to build elegy if the einops dep is failing?

@ndl
Copy link
Contributor Author

ndl commented Jan 10, 2022

einops test issue could be either non-deterministic or dependencies-dependent, hence I guess I got 'lucky' and it built locally the last time? I'm able to reproduce it locally at current nixpkgs master, though.

Logs show that pytorch is stuck at the inference step with multiple hundreds % of CPU usage, this seems similar to https://discuss.pytorch.org/t/pytorch-cpu-hangs-on-nn-linear/17748/4 and setting OMP_NUM_THREADS=1, as suggested in that thread, does seem to help. Added this work-around to this PR, please check if that helps on your side as well.

@ofborg ofborg bot requested a review from yl3dy January 10, 2022 07:35
@samuela
Copy link
Member

samuela commented Jan 10, 2022

Result of nixpkgs-review pr 154115 run on x86_64-linux 1

3 packages failed to build:
  • python310Packages.einops
  • python310Packages.elegy
  • python310Packages.treex
3 packages built:
  • python39Packages.einops
  • python39Packages.elegy
  • python39Packages.treex

@samuela
Copy link
Member

samuela commented Jan 10, 2022

Nicely done! It looks like that fixed it for me. This PR may now supersede #152291. cc @veprbl @yl3dy

Perhaps we should split this out into two PRs though? What does everyone think?

Python 3.10 is still busted but I suspect that's for some dumb reason out of scope here.

Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@yl3dy yl3dy left a comment

Choose a reason for hiding this comment

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

LGTM with respect to einops, thanks a lot!

@samuela
Copy link
Member

samuela commented Jan 10, 2022

ok as long as it looks good to @veprbl and @yl3dy, doing one PR is good with me

@samuela samuela merged commit ae69ff6 into NixOS:master Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants