pythonPackages.fipy: init at 3.3 #65035

wd15 · 2019-07-18T17:53:56Z

Motivation for this change

Add the FiPy PDE solver, see https://www.ctcms.nist.gov/fipy/

Things done

wd15 · 2019-07-18T17:58:09Z

@costrouc: here is the new PR to replace #64643. Let me know what else I need to do.

alexarice

Is it possible to rebase the commits? At the moment the commit for fipy is before its dependency pyamg. Also within the commits there is another python module added and removed again. Not sure this matters in the long run but would be nice to clean up.

pkgs/top-level/all-packages.nix

alexarice · 2019-07-18T18:30:06Z

pkgs/development/python-modules/pysparse/default.nix

+, fetchPypi
+, numpy
+, setuptools
+, blas


Is there any reason this can't be parsed in as openblas to avoid having to pass in { blas = pkgs.openblas } in the callPackage statement?

This is so that in the future someone could provide mkl, atlas, blas instead. We do a similar thing with numpy

fair enough

pkgs/development/python-modules/fipy/default.nix

jonringer · 2019-07-18T19:07:23Z

pkgs/top-level/python-packages.nix

@@ -4044,6 +4046,10 @@ in {

  pyspread = callPackage ../development/python-modules/pyspread { };

+  pysparse = callPackage ../development/python-modules/pysparse {
+    blas = pkgs.openblas;


not familiar with this package, but i would at the very least keep the aliasing consistent

Suggested change

blas = pkgs.openblas;

openblas = pkgs.openblas;

I mentioned this in a previous comment. What if we would like to use mkl in the future? blas is more consistent. (also atlas and blas are options as well)

you can have additional flags such as:

, pkgs , withBlas ? false , withMKL ? false ....

and then later on:

buildInputs = [ ... ] ++ lib.optional withBlas pkgs.openblas ++ lib.optional withMKL pkgs.mkl;

but, you can set them to be true when calling the package

fipy = callPackage .... { withMKL = true; }

really just depends on how to you want to structure injecting those options.

https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/python-packages.nix#L3817

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/tensorflow/default.nix
is a good example as it conditionally enables CudaSupport. And does an "intelligent" look-up of what instruction sets are available for a given architecture.

I'm not sure if you're trying to set a framework to use, or choose between many.

costrouc · 2019-07-18T19:10:31Z

@wd15 please change the title of the PR to pythonPackages.fipy: init at 3.3

costrouc · 2019-07-18T19:11:22Z

PR builds for me

Result of nix-review pr 65035 1

7 package were build:

python27Packages.fipy
python27Packages.pyamg
python27Packages.pysparse
python27Packages.scikit-fmm
python37Packages.fipy
python37Packages.pyamg
python37Packages.scikit-fmm

alexarice · 2019-07-18T20:23:26Z

I would name the maintainer commit "Add wd15 to maintainers.nix" and put it before any of the others

alexarice · 2019-07-18T20:27:18Z

I think it's not usual to merge master into your branch either

smaret · 2019-07-18T21:22:14Z

python27Packages.pysparse fails to build on Darwin with the following error:

builder for '/nix/store/ywgvqx50gz794d6zz6i29md2qz2pr7wf-python2.7-pysparse-1.1.1-dev.drv' failed with exit code 1; last 10 log lines:
      self.calc_info()
    File "/nix/store/0kfl9gvfhazc0m8yak0k4pnr535ww11n-python2.7-numpy-1.16.4/lib/python2.7/site-packages/numpy/distutils/system_info.py", line 1700, in calc_info
      lib = self.has_cblas(info)
    File "/nix/store/0kfl9gvfhazc0m8yak0k4pnr535ww11n-python2.7-numpy-1.16.4/lib/python2.7/site-packages/numpy/distutils/system_info.py", line 1743, in has_cblas
      extra_postargs=info.get('extra_link_args', []))
    File "/nix/store/m96dlplafzcpmy8sbvgkq4r5ny55w4gg-python-2.7.16/lib/python2.7/distutils/ccompiler.py", line 700, in link_executable
      debug, extra_preargs, extra_postargs, None, target_lang)
    File "/nix/store/m96dlplafzcpmy8sbvgkq4r5ny55w4gg-python-2.7.16/lib/python2.7/distutils/unixccompiler.py", line 205, in link
      raise LinkError, msg
  distutils.errors.LinkError: Command "clang /private/var/folders/r2/zwnv7zsd4bd99mqkn4mhqxn800034l/T/nix-build-python2.7-pysparse-1.1.1-dev.drv-0/tmpsvlr5e/private/var/folders/r2/zwnv7zsd4bd99mqkn4mhqxn800034l/T/nix-build-python2.7-pysparse-1.1.1-dev.drv-0/tmpsvlr5e/source.o -L/usr/lib -lblas -o /private/var/folders/r2/zwnv7zsd4bd99mqkn4mhqxn800034l/T/nix-build-python2.7-pysparse-1.1.1-dev.drv-0/tmpsvlr5e/a.out" failed with exit status 1

wd15 · 2019-07-18T21:58:50Z

@alexarice: commits should be clean now

alexarice

Looks good apart from darwin issues

costrouc · 2019-07-23T17:10:50Z

@wd15 remove the [WIP] tag. Looks like this PR ready to go (aside from the darwin issue). If you want you could set platforms = platforms.linux; in meta to limit builds to only linux. Not sure if you have confirmed support for osx

FRidh · 2019-07-23T17:42:11Z

pkgs/development/python-modules/pysparse/default.nix

+
+  propagatedBuildInputs = [
+    numpy
+    blas


Does it need to link to the same blas as is used by numpy? There is the numpy.blas attribute.

Good catch, yes it does.

Now using numpy.blas

wd15 · 2019-07-23T17:45:01Z

@costrouc, I just need to make one more change. I need to remove Gmsh as a dependency on Darwin as it doesn't seem to build and after than have @smaret retest. I updated the PySparse version since smaret review and checked that it built on Darwin so I'll ping smaret to retest once that is done.

wd15 · 2019-07-23T20:53:03Z

@smaret, thanks for your previous test. I've updated the PySparse build to a more recent version, which built for me on Darwin. Could you try again?

jonringer · 2019-07-23T23:29:18Z

I feel like that brokepip.patch is a little heavy at 22k lines...

wd15 · 2019-07-24T13:51:29Z

@jonringer, I can fix that. I should just apply the patch and use the GitHub repository.

wd15 · 2019-07-24T14:28:05Z

@jonringer, large patch is gone

smaret · 2019-07-24T20:01:52Z

@smaret, thanks for your previous test. I've updated the PySparse build to a more recent version, which built for me on Darwin. Could you try again?

PR builds on Darwin for python27. I get an error while building python37.tkinter but that's most likely probably unrelated to this PR. Looks good to me.

costrouc · 2019-07-24T23:05:54Z

Result of nix-review pr 65035 1

7 package were build:

python27Packages.fipy
python27Packages.pyamg
python27Packages.pysparse
python27Packages.scikit-fmm
python37Packages.fipy
python37Packages.pyamg
python37Packages.scikit-fmm

nix review passes! LGTM

wd15 requested a review from FRidh as a code owner July 18, 2019 17:53

ofborg bot added the 6.topic: python label Jul 18, 2019

alexarice reviewed Jul 18, 2019

View reviewed changes

costrouc reviewed Jul 18, 2019

View reviewed changes

pkgs/development/python-modules/fipy/default.nix Outdated Show resolved Hide resolved

jonringer reviewed Jul 18, 2019

View reviewed changes

wd15 changed the title ~~Python fipy init~~ pythonPackages.fipy: init at 3.3 Jul 18, 2019

costrouc approved these changes Jul 18, 2019

View reviewed changes

ofborg bot added the 2.status: merge conflict label Jul 18, 2019

ofborg bot added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 2.status: merge conflict labels Jul 18, 2019

Add wd15 to maintainers.nix

df808ca

wd15 force-pushed the python-fipy-init branch 2 times, most recently from 0c426d1 to 1eb6142 Compare July 18, 2019 21:20

wd15 changed the title ~~pythonPackages.fipy: init at 3.3~~ [WIP] pythonPackages.fipy: init at 3.3 Jul 18, 2019

pythonPackages.scikit-fmm: init at 2019.1.30

9f30e7d

wd15 force-pushed the python-fipy-init branch from 1eb6142 to ef6a2dc Compare July 18, 2019 21:56

alexarice approved these changes Jul 18, 2019

View reviewed changes

wd15 force-pushed the python-fipy-init branch from ef6a2dc to c5d1565 Compare July 19, 2019 15:01

wd15 mentioned this pull request Jul 23, 2019

[WIP] pythonPackages.fipy: init at 3.3 #64643

Closed

10 tasks

FRidh reviewed Jul 23, 2019

View reviewed changes

wd15 force-pushed the python-fipy-init branch from c5d1565 to 0581623 Compare July 23, 2019 20:48

wd15 changed the title ~~[WIP] pythonPackages.fipy: init at 3.3~~ pythonPackages.fipy: init at 3.3 Jul 23, 2019

wd15 changed the title ~~pythonPackages.fipy: init at 3.3~~ [WIP] pythonPackages.fipy: init at 3.3 Jul 24, 2019

costrouc added 3 commits July 24, 2019 10:20

pythonPackages.pysparse: init at 1.1.1-dev

92a3c68

pythonPackages.pyamg: init at 4.0.0

4d007a4

pythonPackages.fipy: init at 3.3

b444004

wd15 force-pushed the python-fipy-init branch from 0581623 to b444004 Compare July 24, 2019 14:21

wd15 changed the title ~~[WIP] pythonPackages.fipy: init at 3.3~~ pythonPackages.fipy: init at 3.3 Jul 24, 2019

wd15 mentioned this pull request Jul 24, 2019

Update the Nix recipe usnistgov/fipy#658

Closed

FRidh merged commit 92e1376 into NixOS:master Jul 28, 2019

wd15 deleted the python-fipy-init branch August 27, 2019 15:14

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pythonPackages.fipy: init at 3.3 #65035

pythonPackages.fipy: init at 3.3 #65035

wd15 commented Jul 18, 2019 •

edited

wd15 commented Jul 18, 2019

alexarice left a comment

alexarice Jul 18, 2019

costrouc Jul 18, 2019

alexarice Jul 18, 2019

jonringer Jul 18, 2019

costrouc Jul 18, 2019

jonringer Jul 18, 2019

costrouc Jul 18, 2019

jonringer Jul 18, 2019

costrouc commented Jul 18, 2019

costrouc commented Jul 18, 2019

alexarice commented Jul 18, 2019

alexarice commented Jul 18, 2019

smaret commented Jul 18, 2019

wd15 commented Jul 18, 2019

alexarice left a comment

costrouc commented Jul 23, 2019

FRidh Jul 23, 2019

wd15 Jul 23, 2019

wd15 Jul 23, 2019

wd15 commented Jul 23, 2019

wd15 commented Jul 23, 2019

jonringer commented Jul 23, 2019

wd15 commented Jul 24, 2019

wd15 commented Jul 24, 2019

smaret commented Jul 24, 2019

costrouc commented Jul 24, 2019

pythonPackages.fipy: init at 3.3 #65035

pythonPackages.fipy: init at 3.3 #65035

Conversation

wd15 commented Jul 18, 2019 • edited

Motivation for this change

Things done

wd15 commented Jul 18, 2019

alexarice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

costrouc commented Jul 18, 2019

costrouc commented Jul 18, 2019

alexarice commented Jul 18, 2019

alexarice commented Jul 18, 2019

smaret commented Jul 18, 2019

wd15 commented Jul 18, 2019

alexarice left a comment

Choose a reason for hiding this comment

costrouc commented Jul 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wd15 commented Jul 23, 2019

wd15 commented Jul 23, 2019

jonringer commented Jul 23, 2019

wd15 commented Jul 24, 2019

wd15 commented Jul 24, 2019

smaret commented Jul 24, 2019

costrouc commented Jul 24, 2019

wd15 commented Jul 18, 2019 •

edited