Some free physics packages added #59
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
guix-science/guix-science!59
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "emedernach/guix-science:CLHEP"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Hello,
I would like to add some free physics packages to guix-science.
Best regards,
Emmanuel Medernach
52e51ccdeeto449050beecHello @jakeforster and @civodul,
The following physics packages has been added to physics.scm:
Cheers,
Emmanuel
449050beecto4c565a6f604c565a6f60to30021af98cHi @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 @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 @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
30021af98ctob4ade9deb4Alright @civodul, here are separated commits per package.
It if is Ok I will continue with the other packages in other files
Emmanuel
Thank you for all this work!
Some packages require minor changes.
Please don't forget to use
guix lintwhen submitting a PR, it will catch many little details.@ -0,0 +96,4 @@;;;; Davix;;(define-public davixThis 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+)))Invisible unicode character spotted by Codeberg.
@ -0,0 +180,4 @@(invoke "sh" "./bootstrap.sh"))))))(home-page "https://www.dcache.org/downloads/dcap/")(synopsis "DCAP")Missing synopsis.
@ -0,0 +181,4 @@(home-page "https://www.dcache.org/downloads/dcap/")(synopsis "DCAP")(description "dCache access protocol client library ")I guess
guix lintcomplained about a missing dot at the end?@ -0,0 +184,4 @@(description "dCache access protocol client library ")(license license:lgpl2.1+)))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))coreutilsandgcc-toolchainshould be removed as they are already pulled by the build system.python-2.7might need to go tonative-inputsif 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")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)")Please split this long line (
guix lintmust have complained about it).@ -0,0 +211,4 @@;;;; TALYS;;(define-public TALYSPlease use lowercase letters for package variable and name.
@ -0,0 +228,4 @@sedtargzipgfortran-toolchain))All these packages are part of the build system, they should not be listed.
gfortran(notgfortran-toolchainwhich is a union package that containsbinutilsand co IIRC) should be listed innative-inputsas it is only needed at build time.@ -0,0 +288,4 @@"tar") "/bin/tar")))(let ((current-dir (getcwd)))(display current-dir)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"Please use
install-fileinstead (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"These should go somewhere in
/share/doc/<package-name>-<version>IIRC.@ -0,0 +312,4 @@)))(delete 'bootstap)If the package builds with the typo, then the
bootstrapdoesn't need to be removed.@ -0,0 +317,4 @@(delete 'build)(delete 'install))))Lonely parenthesis. Please run
guix lint <package-name>.@ -0,0 +320,4 @@)))(home-page "https://nds.iaea.org/talys/")(synopsis "TALYS")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)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-systemmight not be the best build system for this. IMHO,gnu-build-systemmight be better suited since it comes with variouspatch-shebangphases.@ -0,0 +340,4 @@(sha256(base32 "1k9x9rcbqg9xax5z8w1fglycf5z8wiaym5kp3naia6m9ny959i3z"))))(build-system trivial-build-system)(inputs (list gzip tar))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"Please use
install-file.@ -0,0 +383,4 @@))))(home-page "https://sourceforge.net/projects/timing-gen/")(synopsis "Timing Gen")Missing synopsis.
@ -0,0 +390,4 @@(license license:expat)))(define-public UnuranPlease use lowercase for package variable and name.
@ -0,0 +402,4 @@(sha256(base32 "0jc0vna3xgdbbw5naixdqm45z1q8ajj4ig4qgqn4q2sr9j2r71q9"))))(build-system gnu-build-system)(inputs (list perl))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")))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 FASTERacPlease use lowercase for package variable and name.
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
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.
Ok then I will use lowercase names
Emmanuel
Sorry I have a question about
guix lint, I have this error and I don't know how to solve it:The package is currently
`(,util-linux "lib")Thanks in advance for your help
Emmanuel
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.Ok Thanks
396cd1fa92to1cb217717eI suggest adding and reviewing one package at a time.
I have added davix, please tell me if there are any problem.
Emmanuel
Here is my review for the
davixpackage. It could also be a good idea to runguix style -f guix-science/packages/physics.scmfor 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-makeUnused imports should also be removed.
@ -67,0 +95,4 @@(sha256(base32 "1w0alvkhc23iy42kqrb543jja4v6xs7z600vhn890hprj3fmpis2"))))(build-system cmake-build-system)(inputsI successfully built this package using only
libxml2,curl,opensslandutil-linux:libas inputs (pythonbeing declared innative-inputs). Is there a specific reason to keep the extra dependencies?@ -67,0 +99,4 @@(list boostcoreutilscurlgcc-toolchain-8coreutilsis 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-toolchainpackage should be declared innative-inputs. See comment in previous review.@ -67,0 +102,4 @@gcc-toolchain-8libxml2openssl-1.1python-2.7Why specifying package version? I successfully built this package with default
opensslandpythonpackages. If there is no specific reason to used an outdated version, the default package should be used.Also,
pythonmight need to go tonative-inputssince it seems only used to build the package.@ -67,0 +104,4 @@openssl-1.1python-2.7`(,util-linux "lib") ; For <uuid/uuid.h>util-linux+udev))Why adding both
util-linuxandutil-linux+udev? FTR, I successfully builtdavixwithout any dependency toutil-linux+udev.@ -67,0 +107,4 @@util-linux+udev))(arguments`(#:phasesI 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
#:phasesargument, this would become: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"Due to the quote, this bloc is not executed. It should be removed (I successfully built this package without this bloc).
1cb217717etoca0a18bbbbHi @rgarbage
Thanks for your review, I make the corresponding corrections.
Please tell me if there are any remaining problem.
Emmanuel
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?
Thanks, yes I will add one package at a time in this branch.
Emmanuel
I merged it as commit
e6fc8d1cd3with some minor edits of the module header and the commit message.Thanks for all the work @emedernach & @rgarbage!
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).
Oh Ok Thanks I will do it.