Some free physics packages added #59

Manually merged
rgarbage merged 1 commit from emedernach/guix-science:CLHEP into master 2025-01-16 09:56:11 +01:00
Contributor

Hello,

I would like to add some free physics packages to guix-science.

Best regards,

Emmanuel Medernach

Hello, I would like to add some free physics packages to guix-science. Best regards, Emmanuel Medernach
Author
Contributor

Hello @jakeforster and @civodul,

The following physics packages has been added to physics.scm:

  • dcap
  • davix
  • vdt
  • TALYS
  • timing-gen
  • Unuran
  • FASTERac

Cheers,
Emmanuel

Hello @jakeforster and @civodul, The following physics packages has been added to physics.scm: - dcap - davix - vdt - TALYS - timing-gen - Unuran - FASTERac Cheers, Emmanuel
Owner

Hi @emedernach,

Thanks a lot for this! As mentioned earlier, we'll probably have to split it in smaller bites: typically one commit per package, and ideally one merge request per package.

Maybe the extra work can be shared between @jakefoster and yourself, let's see.

At any rate, this a much welcome contribution. Looks like guix-science could become a useful place for physicists. :-)

Ludo'.

Hi @emedernach, Thanks a lot for this! As mentioned earlier, we'll probably have to split it in smaller bites: typically one commit per package, and ideally one merge request per package. Maybe the extra work can be shared between @jakefoster and yourself, let's see. At any rate, this a much welcome contribution. Looks like guix-science could become a useful place for physicists. :-) Ludo'.
Author
Contributor

Hi @civodul,

Ok I will do a merge request per package then.

I think we could close this one.

Emmanuel

Hi @civodul, Ok I will do a merge request per package then. I think we could close this one. Emmanuel
Owner

Hi @civodul,

Ok I will do a merge request per package then.

I think we could close this one.

Emmanuel

Hi @emedernach

You don't necessarily have to close this one, you can also rewrite the git history of the branch associated with this pull request to split the existing commit into a series of commits, where each commit adds a new package. HTH.

> Hi @civodul, > > Ok I will do a merge request per package then. > > I think we could close this one. > > Emmanuel Hi @emedernach You don't necessarily have to close this one, you can also rewrite the git history of the branch associated with this pull request to split the existing commit into a series of commits, where each commit adds a new package. HTH.
Author
Contributor

HI @rgarbage

Oh Ok Thanks, @civodul message was "ideally one merge request per package". I will split the commit into multiple commits, one per package then.

Emmanuel

HI @rgarbage Oh Ok Thanks, @civodul message was "ideally one merge request per package". I will split the commit into multiple commits, one per package then. Emmanuel
Owner

HI @rgarbage

Oh Ok Thanks, @civodul message was "ideally one merge request per package". I will split the commit into multiple commits, one per package then.

Emmanuel

@emedernach Sorry I misread @civodul post.

It is indeed a better practice to submit a pull request per package.

IMHO, if the number of newly added packages is kept small, it is fine.

It is also a common practice to submit a pull request with multiple packages where the "target package" requires dependencies that don't exist yet in the channel.

In this particular case, I can't say as I didn't review the PR. Feel free to do as it suits you better, the important thing here is having separated commits per package.

Romain

> HI @rgarbage > > Oh Ok Thanks, @civodul message was "ideally one merge request per package". I will split the commit into multiple commits, one per package then. > > Emmanuel > @emedernach Sorry I misread @civodul post. It is indeed a better practice to submit a pull request per package. IMHO, if the number of newly added packages is kept small, it is fine. It is also a common practice to submit a pull request with multiple packages where the "target package" requires dependencies that don't exist yet in the channel. In this particular case, I can't say as I didn't review the PR. Feel free to do as it suits you better, the important thing here is having separated commits per package. Romain
Author
Contributor

Alright @civodul, here are separated commits per package.

It if is Ok I will continue with the other packages in other files

Emmanuel

Alright @civodul, here are separated commits per package. It if is Ok I will continue with the other packages in other files Emmanuel
rgarbage left a comment
Owner

Thank you for all this work!

Some packages require minor changes.

Please don't forget to use guix lint when submitting a PR, it will catch many little details.

Thank you for all this work! Some packages require minor changes. Please don't forget to use `guix lint` when submitting a PR, it will catch many little details.
@ -0,0 +96,4 @@
;;
;; Davix
;;
(define-public davix
Owner

This commit doesn't add only one package. Maybe you need to rebase to latest HEAD of the default branch?

This commit doesn't add only one package. Maybe you need to rebase to latest HEAD of the default branch?
@ -0,0 +143,4 @@
HTTP-based protocols simple.")
(license license:lgpl2.1+)))
Owner

Invisible unicode character spotted by Codeberg.

Invisible unicode character spotted by Codeberg.
@ -0,0 +180,4 @@
(invoke "sh" "./bootstrap.sh"))))))
(home-page "https://www.dcache.org/downloads/dcap/")
(synopsis "DCAP")
Owner

Missing synopsis.

Missing synopsis.
@ -0,0 +181,4 @@
(home-page "https://www.dcache.org/downloads/dcap/")
(synopsis "DCAP")
(description "dCache access protocol client library ")
Owner

I guess guix lint complained about a missing dot at the end?

I guess `guix lint` complained about a missing dot at the end?
@ -0,0 +184,4 @@
(description "dCache access protocol client library ")
(license license:lgpl2.1+)))
Owner

Invisible unicode character spotted by Codeberg.

Invisible unicode character spotted by Codeberg.
@ -0,0 +198,4 @@
(sha256
(base32 "16746jfcpwaxngmqwg65barrpsiajvfp3lc95wal4868bss685lb"))))
(build-system cmake-build-system)
(inputs (list coreutils gcc-toolchain python-2.7))
Owner

coreutils and gcc-toolchain should be removed as they are already pulled by the build system.
python-2.7 might need to go to native-inputs if it is only needed at build time.

`coreutils` and `gcc-toolchain` should be removed as they are already pulled by the build system. `python-2.7` might need to go to `native-inputs` if it is only needed at build time.
@ -0,0 +202,4 @@
(arguments
`(#:tests? #f)) ;There are no tests defined in VDT
(home-page "https://root.cern/root/html606/md_math_vdt_ReadMe.html")
(synopsis "VDT")
Owner

Please expand the synopsis.

Please expand the synopsis.
@ -0,0 +204,4 @@
(home-page "https://root.cern/root/html606/md_math_vdt_ReadMe.html")
(synopsis "VDT")
(description
"VDT is a library of mathematical functions, implemented in double and single precision. The implementation is fast and vectorisable with the aid of modern compilers. VDT exploits also Pade polynomials. A lot of ideas were inspired by the cephes math library (by Stephen L. Moshier)")
Owner

Please split this long line (guix lint must have complained about it).

Please split this long line (`guix lint` must have complained about it).
@ -0,0 +211,4 @@
;;
;; TALYS
;;
(define-public TALYS
Owner

Please use lowercase letters for package variable and name.

Please use lowercase letters for package variable and name.
@ -0,0 +228,4 @@
sed
tar
gzip
gfortran-toolchain))
Owner

All these packages are part of the build system, they should not be listed.
gfortran (not gfortran-toolchain which is a union package that contains binutils and co IIRC) should be listed in native-inputs as it is only needed at build time.

All these packages are part of the build system, they should not be listed. `gfortran` (not `gfortran-toolchain` which is a union package that contains `binutils` and co IIRC) should be listed in `native-inputs` as it is only needed at build time.
@ -0,0 +288,4 @@
"tar") "/bin/tar")))
(let ((current-dir (getcwd)))
(display current-dir)
Owner

These look like debug statements. IMHO they should be removed to keep the package definition as concise as possible.

These look like debug statements. IMHO they should be removed to keep the package definition as concise as possible.
@ -0,0 +294,4 @@
(chdir current-dir))
(mkdir-p (string-append out "/bin"))
(copy-file "bin/talys"
Owner
Please use `install-file` instead (https://guix.gnu.org/manual/devel/en/guix.html#index-install_002dfile).
@ -0,0 +305,4 @@
dir)))
'("doc" "samples" "structure" "source")))
(copy-file "README.md"
Owner

These should go somewhere in /share/doc/<package-name>-<version> IIRC.

These should go somewhere in `/share/doc/<package-name>-<version>` IIRC.
@ -0,0 +312,4 @@
)))
(delete 'bootstap)
Owner

If the package builds with the typo, then the bootstrap doesn't need to be removed.

If the package builds with the typo, then the `bootstrap` doesn't need to be removed.
@ -0,0 +317,4 @@
(delete 'build)
(delete 'install)
)))
Owner

Lonely parenthesis. Please run guix lint <package-name>.

Lonely parenthesis. Please run `guix lint <package-name>`.
@ -0,0 +320,4 @@
)))
(home-page "https://nds.iaea.org/talys/")
(synopsis "TALYS")
Owner

Missing synopsis.

Missing synopsis.
@ -0,0 +339,4 @@
"https://sourceforge.net/projects/timing-gen/files/timing-gen-0.9.8.tgz")
(sha256
(base32 "1k9x9rcbqg9xax5z8w1fglycf5z8wiaym5kp3naia6m9ny959i3z"))))
(build-system trivial-build-system)
Owner

According to https://sourceforge.net/projects/timing-gen/files/ (README content), this package depends on PERL and other libraries that should be listed as inputs.

trivial-build-system might not be the best build system for this. IMHO, gnu-build-system might be better suited since it comes with various patch-shebang phases.

According to https://sourceforge.net/projects/timing-gen/files/ (README content), this package depends on PERL and other libraries that should be listed as `inputs`. `trivial-build-system` might not be the best build system for this. IMHO, `gnu-build-system` might be better suited since it comes with various `patch-shebang` phases.
@ -0,0 +340,4 @@
(sha256
(base32 "1k9x9rcbqg9xax5z8w1fglycf5z8wiaym5kp3naia6m9ny959i3z"))))
(build-system trivial-build-system)
(inputs (list gzip tar))
Owner

These should be native-inputs (used at build time).

These should be `native-inputs` (used at build time).
@ -0,0 +375,4 @@
(string-append out "/share/doc"))
(copy-recursively "samples"
(string-append out "/share/samples"))
(copy-file "timing-gen"
Owner

Please use install-file.

Please use `install-file`.
@ -0,0 +383,4 @@
))))
(home-page "https://sourceforge.net/projects/timing-gen/")
(synopsis "Timing Gen")
Owner

Missing synopsis.

Missing synopsis.
@ -0,0 +390,4 @@
(license license:expat)))
(define-public Unuran
Owner

Please use lowercase for package variable and name.

Please use lowercase for package variable and name.
@ -0,0 +402,4 @@
(sha256
(base32 "0jc0vna3xgdbbw5naixdqm45z1q8ajj4ig4qgqn4q2sr9j2r71q9"))))
(build-system gnu-build-system)
(inputs (list perl))
Owner

I couldn't find a mention of PERL in https://statmath.wu.ac.at/unuran/doc/unuran.html#Installation . If needed at build time, it should go to native-inputs.

I couldn't find a mention of PERL in https://statmath.wu.ac.at/unuran/doc/unuran.html#Installation . If needed at build time, it should go to `native-inputs`.
@ -0,0 +405,4 @@
(inputs (list perl))
(arguments
'(#:configure-flags '("--with-pic")))
Owner

Also, according to https://statmath.wu.ac.at/unuran/doc/unuran.html#Installation, the recommended URNG library is http://statmath.wu.ac.at/software/RngStreams/, and the following flags should be set: --with-urng-rngstream --with-urng-default=rngstream. Is there a particular reason it is not the case in this package?

Also, according to https://statmath.wu.ac.at/unuran/doc/unuran.html#Installation, the recommended URNG library is http://statmath.wu.ac.at/software/RngStreams/, and the following flags should be set: `--with-urng-rngstream --with-urng-default=rngstream`. Is there a particular reason it is not the case in this package?
@ -0,0 +416,4 @@
(license license:gpl2)))
(define-public FASTERac
Owner

Please use lowercase for package variable and name.

Please use lowercase for package variable and name.
Author
Contributor

Thanks for your detailed review I will make the corresponding changes.

One note though, TALYS software name is uppercase, they are known with uppercase names such as other packages (FASTERac or ROOT). Could we keep the uppercase names ?

Emmanuel

Thanks for your detailed review I will make the corresponding changes. One note though, TALYS software name is uppercase, they are known with uppercase names such as other packages (FASTERac or ROOT). Could we keep the uppercase names ? Emmanuel
Owner

Thanks for your detailed review I will make the corresponding changes.

One note though, TALYS software name is uppercase, they are known with uppercase names such as other packages (FASTERac or ROOT). Could we keep the uppercase names ?

The current practice is to have both package variable and name written in lowercase.

See for example PETSc which is a well known HPC library (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/maths.scm#n3453).

I don't know of any exception to this rule. IMHO it should be enforced for consistency.

> Thanks for your detailed review I will make the corresponding changes. > > One note though, TALYS software name is uppercase, they are known with uppercase names such as other packages (FASTERac or ROOT). Could we keep the uppercase names ? The current practice is to have both package variable and name written in lowercase. See for example PETSc which is a well known HPC library (https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/maths.scm#n3453). I don't know of any exception to this rule. IMHO it should be enforced for consistency.
Author
Contributor

Ok then I will use lowercase names

Emmanuel

Ok then I will use lowercase names Emmanuel
Author
Contributor

Sorry I have a question about guix lint, I have this error and I don't know how to solve it:

[email protected]: label 'util-linux' does not match package name 'util-linux:lib'

The package is currently `(,util-linux "lib")

Thanks in advance for your help

Emmanuel

Sorry I have a question about `guix lint`, I have this error and I don't know how to solve it: > [email protected]: label 'util-linux' does not match package name 'util-linux:lib' The package is currently `` `(,util-linux "lib") `` Thanks in advance for your help Emmanuel
Owner

Sorry I have a question about guix lint, I have this error and I don't know how to solve it:

[email protected]: label 'util-linux' does not match package name 'util-linux:lib'

The package is currently `(,util-linux "lib")

This seems related to the fact that the "new" way of declaring inputs automatically generates a label which is not the label expected by guix lint. IMHO this is fine to ignore this warning.

> Sorry I have a question about `guix lint`, I have this error and I don't know how to solve it: > > > [email protected]: label 'util-linux' does not match package name 'util-linux:lib' > > The package is currently `` `(,util-linux "lib") `` This seems related to the fact that the "new" way of declaring inputs automatically generates a label which is not the label expected by `guix lint`. IMHO this is fine to ignore this warning.
Author
Contributor

Ok Thanks

Ok Thanks
Author
Contributor

I suggest adding and reviewing one package at a time.

I have added davix, please tell me if there are any problem.

Emmanuel

I suggest adding and reviewing one package at a time. I have added davix, please tell me if there are any problem. Emmanuel
rgarbage left a comment
Owner

Here is my review for the davix package. It could also be a good idea to run guix style -f guix-science/packages/physics.scm for proper code formatting, but beware that it modifies files in-place.

HTH,
Romain

Here is my review for the `davix` package. It could also be a good idea to run `guix style -f guix-science/packages/physics.scm` for proper code formatting, but beware that it modifies files in-place. HTH, Romain
@ -18,2 +18,4 @@
(define-module (guix-science packages physics)
#:use-module ((guix licenses) #:prefix license:)
#:use-module (gnu packages base) ; coreutils gnu-make
Owner

Unused imports should also be removed.

Unused imports should also be removed.
@ -67,0 +95,4 @@
(sha256
(base32 "1w0alvkhc23iy42kqrb543jja4v6xs7z600vhn890hprj3fmpis2"))))
(build-system cmake-build-system)
(inputs
Owner

I successfully built this package using only libxml2, curl, openssl and util-linux:lib as inputs (python being declared in native-inputs). Is there a specific reason to keep the extra dependencies?

I successfully built this package using only `libxml2`, `curl`, `openssl` and `util-linux:lib` as inputs (`python` being declared in `native-inputs`). Is there a specific reason to keep the extra dependencies?
@ -67,0 +99,4 @@
(list boost
coreutils
curl
gcc-toolchain-8
Owner

coreutils is pulled by the build system, it should never be declared in the inputs.
Same goes for gcc-toolchain, in any case, if a different GCC version is needed (or another compiler), the non -toolchain package should be declared in native-inputs. See comment in previous review.

`coreutils` is pulled by the build system, it should never be declared in the inputs. Same goes for `gcc-toolchain`, in any case, if a different GCC version is needed (or another compiler), the non `-toolchain` package should be declared in `native-inputs`. See comment in previous review.
@ -67,0 +102,4 @@
gcc-toolchain-8
libxml2
openssl-1.1
python-2.7
Owner

Why specifying package version? I successfully built this package with default openssl and python packages. If there is no specific reason to used an outdated version, the default package should be used.
Also, python might need to go to native-inputs since it seems only used to build the package.

Why specifying package version? I successfully built this package with default `openssl` and `python` packages. If there is no specific reason to used an outdated version, the default package should be used. Also, `python` might need to go to `native-inputs` since it seems only used to build the package.
@ -67,0 +104,4 @@
openssl-1.1
python-2.7
`(,util-linux "lib") ; For <uuid/uuid.h>
util-linux+udev))
Owner

Why adding both util-linux and util-linux+udev? FTR, I successfully built davix without any dependency to util-linux+udev.

Why adding both `util-linux` and `util-linux+udev`? FTR, I successfully built `davix` without any dependency to `util-linux+udev`.
@ -67,0 +107,4 @@
util-linux+udev))
(arguments
`(#:phases
Owner

I would suggest switching to G-expressions for better readability (see https://guix.gnu.org/manual/devel/en/guix.html#index-G_002dexpression).

With the removal of the #:phases argument, this would become:

(arguments
   (list #:configure-flags
          #~(list "-DEMBEDDED_LIBCURL=FALSE"
                      "-DLIBCURL_BACKEND_BY_DEFAULT=TRUE")))

Don't forget to add ":use-module (guix gexp) in the module declaration.

I would suggest switching to G-expressions for better readability (see https://guix.gnu.org/manual/devel/en/guix.html#index-G_002dexpression). With the removal of the `#:phases` argument, this would become: ``` (arguments (list #:configure-flags #~(list "-DEMBEDDED_LIBCURL=FALSE" "-DLIBCURL_BACKEND_BY_DEFAULT=TRUE"))) ``` Don't forget to add `":use-module (guix gexp)` in the module declaration.
@ -67,0 +113,4 @@
(add-before
'configure 'patch
(lambda args
'(substitute* "src/fileops/AzureIO.cpp"
Owner

Due to the quote, this bloc is not executed. It should be removed (I successfully built this package without this bloc).

Due to the quote, this bloc is not executed. It should be removed (I successfully built this package without this bloc).
Author
Contributor

Hi @rgarbage

Thanks for your review, I make the corresponding corrections.

Please tell me if there are any remaining problem.

Emmanuel

Hi @rgarbage Thanks for your review, I make the corresponding corrections. Please tell me if there are any remaining problem. Emmanuel
Owner

Hi @rgarbage

Thanks for your review, I make the corresponding corrections.

Please tell me if there are any remaining problem.

Hi @emedernach

Looks good, thanks for the update. I'll merge it now, so you'll submit other merge requests for the remaining packages, is that your plan?

> Hi @rgarbage > > Thanks for your review, I make the corresponding corrections. > > Please tell me if there are any remaining problem. Hi @emedernach Looks good, thanks for the update. I'll merge it now, so you'll submit other merge requests for the remaining packages, is that your plan?
Author
Contributor

Thanks, yes I will add one package at a time in this branch.

Emmanuel

Thanks, yes I will add one package at a time in this branch. Emmanuel
rgarbage manually merged commit e6fc8d1cd3 into master 2025-01-16 09:56:11 +01:00
Owner

I merged it as commit e6fc8d1cd3 with some minor edits of the module header and the commit message.

I merged it as commit e6fc8d1cd319ecd49045d6fec71d0c11d7130ade with some minor edits of the module header and the commit message.
Owner

Thanks for all the work @emedernach & @rgarbage!

Thanks for all the work @emedernach & @rgarbage!
Author
Contributor

Hi @civodul and @rgarbage ,

I have added the dcap package, please tell me if this is Ok

Emmanuel

Hi @civodul and @rgarbage , I have added the dcap package, please tell me if this is Ok Emmanuel
Owner

Hi @civodul and @rgarbage ,

I have added the dcap package, please tell me if this is Ok

Emmanuel

Hi @emedernach

Since that pull request has been merged, you should open a new one (with the same branch in your fork, it doesn't matter).

> Hi @civodul and @rgarbage , > > I have added the dcap package, please tell me if this is Ok > > Emmanuel Hi @emedernach Since that pull request has been merged, you should open a new one (with the same branch in your fork, it doesn't matter).
Author
Contributor

Oh Ok Thanks I will do it.

Oh Ok Thanks I will do it.
Sign in to join this conversation.
No description provided.