Message ID | 20221223011551.32708-1-antero@mailbox.org |
---|---|
State | New |
Headers | show |
Series | [bug#60266] gnu: Add form. | expand |
Hello Antero, Thank you for the patch. Sorry for the delayed reply, holidays and such. I was able to apply your patch and build a form package. A few comments: On Fri, 2022-12-23 at 01:15 +0000, Antero Mejr wrote: > * gnu/packages/maths.scm (form): New variable. > --- > x86_64 only due to test failures on other platforms. Developers > say other platforms are not "tier 1" supported: > https://github.com/vermaseren/form/issues/426 This may be better as a comment near the `supported-systems` field, along with a short summary of which tests fail on other systems. > > gnu/packages/maths.scm | 55 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm > index 050450e12c..08ddd2ecb4 100644 > --- a/gnu/packages/maths.scm > +++ b/gnu/packages/maths.scm > @@ -8161,3 +8161,58 @@ (define-public optizelle > provided for applications written in C++ and Python. Parallel > computation is supported via MPI.") > (license license:bsd-2)))) > + > +(define-public form > + (let ((commit "28e15eaf0856a0a012795298d6a4b570e764a8b1") This commit is downstream from the 4.3.0 release, so we should include in a comment the rationale for not using just the 4.3.0 release tarball. > + (revision "0")) > + (package > + (name "form") > + (version (git-version "4.3.0" revision commit)) > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url "https://github.com/vermaseren/form") > + (commit commit))) > + (file-name (git-file-name name version)) > + (sha256 > + (base32 > + "04i932lqwng2hmknvai1gmmb5j17rwrhlj11nr96w9bmj4sq736x")) > + (modules '((guix build utils))) > + (snippet #~(substitute* "check/examples.frm" > + (("#pend_if valgrind\\?") > + "#pend_if 0"))))) This snippet appears to be related to the specifics of this package build? E.g. if someone were to grab the source with `guix build -S form`, they would not be able to have valgrind support, correct? If that's the case, perhaps it would be better to apply this substitution in a build phase. > + (build-system gnu-build-system) > + (native-inputs (list autoconf automake doxygen ruby)) > + (inputs (list bash openmpi)) > + (arguments > + (list #:configure-flags #~(list "--enable-parform") For some other maths packages that have both a serial and MPI versions, we've usually provided two packages. See e.g. `petsc` and `petsc- openmpi`. This can be useful if someone does not need a full MPI- capable version of form, and the separation is easily achieved. Also, I see the default compilation flags include `-march=native`, which will most likely cause a problem with build farm substitutes or `guix challenge`. Could you figure out have to override these flags? Guix usually assumes `SSE2` capabilities for x86_64 targets, iirc. Bonus points for enabling a "tunable" package (c.f. "Package Transformation Options"). > + #:phases #~(modify-phases %standard-phases > + (add-after 'unpack 'fix-hardcoded-path > + (lambda _ > + (substitute* "sources/extcmd.c" > + (("/bin/sh") > + (string-append #$(this-package-input "bash") > + "/bin/sh"))))) > + (add-after 'build 'build-doxygen > + (lambda _ > + (with-directory-excursion "doc/doxygen" > + (invoke "make" "html")))) > + (add-before 'check 'mpi-setup > + #$%openmpi-setup) > + (add-after 'install 'install-docs > + (lambda _ > + (let ((doc (string-append #$output "/share/doc/" > + #$name "-" #$version > + "/html"))) > + (mkdir-p doc) > + (copy-recursively "doc/doxygen/html" doc))))))) > + (home-page "https://www.nikhef.nl/~form/maindir/maindir.html") Maybe we should use https://www.nikhef.nl/~form/ instead? This is the URL specified in the included manpage. > + (synopsis "Symbolic manipulation system for very big expressions") > + (description > + "FORM is a symbolic manipulation system. It reads symbolic expressions > +from files and executes symbolic/algebraic transformations upon them. The > +answers are returned in a textual mathematical representation. The size of the > +considered expressions in FORM is only limited by the available disk space and > +not by the available RAM.") > + (supported-systems '("x86_64-linux")) > + (license license:gpl3+)))) Thanks, `~Eric
Hello, I think this package should be in algebra.scm, not maths.scm. The names are not completely logical, actually maths is all maths that is not algebra (and a few packages are already in the maths module although they should not be). Andreas
Eric Bavier <bavier@posteo.net> writes: >> x86_64 only due to test failures on other platforms. Developers >> say other platforms are not "tier 1" supported: >> https://github.com/vermaseren/form/issues/426 > > This may be better as a comment near the `supported-systems` field, > along with a short summary of which tests fail on other systems. Added comments in v2. > This commit is downstream from the 4.3.0 release, so we should include > in a comment the rationale for not using just the 4.3.0 release > tarball. Added rationale. >> + (snippet #~(substitute* "check/examples.frm" >> + (("#pend_if valgrind\\?") >> + "#pend_if 0"))))) > > This snippet appears to be related to the specifics of this package > build? E.g. if someone were to grab the source with `guix build -S > form`, they would not be able to have valgrind support, correct? If > that's the case, perhaps it would be better to apply this substitution > in a build phase. No, it's disabling a failing test that has a known problem (memory leak). Added a comment with the explanation. > For some other maths packages that have both a serial and MPI versions, > we've usually provided two packages. See e.g. `petsc` and `petsc- > openmpi`. This can be useful if someone does not need a full MPI- > capable version of form, and the separation is easily achieved. Moved the openMPI version to the parform package. > Also, I see the default compilation flags include `-march=native`, > which will most likely cause a problem with build farm substitutes or > `guix challenge`. Could you figure out have to override these flags? > Guix usually assumes `SSE2` capabilities for x86_64 targets, iirc. > Bonus points for enabling a "tunable" package (c.f. "Package > Transformation Options"). Disabled --march=native in v2. > Maybe we should use https://www.nikhef.nl/~form/ instead? This is the > URL specified in the included manpage. I used the other homepage so people can skip the weird "license agreement" page at https://www.nikhef.nl/~form/. But I changed it to your suggestion in v2. Thanks for the review.
Am Donnerstag, dem 16.03.2023 um 01:15 +0000 schrieb Antero Mejr: > > > + (snippet #~(substitute* "check/examples.frm" > > > + (("#pend_if valgrind\\?") > > > + "#pend_if 0"))))) > > > > This snippet appears to be related to the specifics of this package > > build? E.g. if someone were to grab the source with `guix build -S > > form`, they would not be able to have valgrind support, correct? > > If that's the case, perhaps it would be better to apply this > > substitution in a build phase. > > No, it's disabling a failing test that has a known problem (memory > leak). Added a comment with the explanation. Test failures should also be fixed at build time. Other than that, you could also try to fix the memory leak that causes the failure via a patch. > > Guix usually assumes `SSE2` capabilities for x86_64 targets, iirc. It does, but not for i386 to i686, which also often see sse2 flags in build systems. > > I used the other homepage so people can skip the weird "license > agreement" page at https://www.nikhef.nl/~form/. Calling the GPL a license agreement is weird, and it doesn't help that https://www.nikhef.nl/~form/maindir/ also refers to it. Perhaps we can raise an issue about that upstream? Cheers
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > Test failures should also be fixed at build time. Other than that, you > could also try to fix the memory leak that causes the failure via a > patch. Fixed the test failure at build time in v3. >> > Guix usually assumes `SSE2` capabilities for x86_64 targets, iirc. > It does, but not for i386 to i686, which also often see sse2 flags in > build systems. I did some benchmarks on a few workloads with millions of terms. The "tunable" optimizations didn't consistently improve the performance. Sometimes it would be a couple percent faster, sometimes slower. So I didn't mark the package as tunable. >> I used the other homepage so people can skip the weird "license >> agreement" page at https://www.nikhef.nl/~form/. > Calling the GPL a license agreement is weird, and it doesn't help that > https://www.nikhef.nl/~form/maindir/ also refers to it. Perhaps we can > raise an issue about that upstream? I would prefer not to bother upstream further regarding the website, since I already had to ask them to change an invalid license statement in a file header (which they did).
diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm index 050450e12c..08ddd2ecb4 100644 --- a/gnu/packages/maths.scm +++ b/gnu/packages/maths.scm @@ -8161,3 +8161,58 @@ (define-public optizelle provided for applications written in C++ and Python. Parallel computation is supported via MPI.") (license license:bsd-2)))) + +(define-public form + (let ((commit "28e15eaf0856a0a012795298d6a4b570e764a8b1") + (revision "0")) + (package + (name "form") + (version (git-version "4.3.0" revision commit)) + (source (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/vermaseren/form") + (commit commit))) + (file-name (git-file-name name version)) + (sha256 + (base32 + "04i932lqwng2hmknvai1gmmb5j17rwrhlj11nr96w9bmj4sq736x")) + (modules '((guix build utils))) + (snippet #~(substitute* "check/examples.frm" + (("#pend_if valgrind\\?") + "#pend_if 0"))))) + (build-system gnu-build-system) + (native-inputs (list autoconf automake doxygen ruby)) + (inputs (list bash openmpi)) + (arguments + (list #:configure-flags #~(list "--enable-parform") + #:phases #~(modify-phases %standard-phases + (add-after 'unpack 'fix-hardcoded-path + (lambda _ + (substitute* "sources/extcmd.c" + (("/bin/sh") + (string-append #$(this-package-input "bash") + "/bin/sh"))))) + (add-after 'build 'build-doxygen + (lambda _ + (with-directory-excursion "doc/doxygen" + (invoke "make" "html")))) + (add-before 'check 'mpi-setup + #$%openmpi-setup) + (add-after 'install 'install-docs + (lambda _ + (let ((doc (string-append #$output "/share/doc/" + #$name "-" #$version + "/html"))) + (mkdir-p doc) + (copy-recursively "doc/doxygen/html" doc))))))) + (home-page "https://www.nikhef.nl/~form/maindir/maindir.html") + (synopsis "Symbolic manipulation system for very big expressions") + (description + "FORM is a symbolic manipulation system. It reads symbolic expressions +from files and executes symbolic/algebraic transformations upon them. The +answers are returned in a textual mathematical representation. The size of the +considered expressions in FORM is only limited by the available disk space and +not by the available RAM.") + (supported-systems '("x86_64-linux")) + (license license:gpl3+))))