Bug 659896 (cp2k) - Review Request: cp2k - A molecular dynamics engine capable of classical and Car-Parrinello simulations
Summary: Review Request: cp2k - A molecular dynamics engine capable of classical and C...
Keywords:
Status: CLOSED ERRATA
Alias: cp2k
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: makedepf90
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-12-04 00:41 UTC by Dominik 'Rathann' Mierzejewski
Modified: 2011-09-18 18:22 UTC (History)
4 users (show)

Fixed In Version: cp2k-2.1-2.20101006.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-12-26 19:53:32 UTC
Type: ---
susi.lehtola: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Dominik 'Rathann' Mierzejewski 2010-12-04 00:41:04 UTC
Spec URL: http://rathann.fedorapeople.org/review/cp2k.spec
SRPM URL: http://rathann.fedorapeople.org/review/cp2k-2.1-1.20101006.fc12.src.rpm
Description:
CP2K is a freely available (GPL) program, written in Fortran 95, to
perform atomistic and molecular simulations of solid state, liquid,
molecular and biological systems. It provides a general framework for
different methods such as e.g. density functional theory (DFT) using a
mixed Gaussian and plane waves approach (GPW), and classical pair and
many-body potentials.

Comment 1 Susi Lehtola 2010-12-04 23:13:20 UTC
Hmm, I've meant to package this for some time, although I haven't been using it myself for the time being. In the past one of the problems was that there were no stable releases. This seems to have changed somewhat as they now have a stable branch. However, one might have to be careful in picking the snapshots.

I would put the OpenMP parallelized version in the main package, myself, as it can be ran in single thread mode anyhow.

Another thing is that OpenMP parallellism is usually not as efficient as MPI parallellism on NUMA architectures due to the used memory not being optimally located. Getting the MPI parallellized version to build, however, may not be a trivial task, e.g., as it needs scalapack.

*Please* don't build against reference LAPACK, you'll get an order more throughput by building against ATLAS. Replace BR: lapack-devel and BR: blas-devel with BR: atlas-devel. You'll need to pass "-L%{_libdir}/atlas" as LDFLAGS and "-lf77blas -latlas" as the BLAS library and "-llapack" as the LAPACK library.


I'll go through the review during the weekend.

Comment 2 Susi Lehtola 2010-12-04 23:31:46 UTC
The summmary would IMHO be more descriptive as
 A molecular dynamics engine capable of classical and Car-Parrinello simulations
instead of the plain
 Molecular simulations software

Comment 3 Susi Lehtola 2010-12-06 14:05:56 UTC
rpmlint output:
cp2k.x86_64: W: no-documentation
cp2k.x86_64: W: no-manual-page-for-binary cp2k.sopt
cp2k-smp.x86_64: W: spelling-error Summary(en_US) multi -> mulch, mufti
cp2k-smp.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti
cp2k-smp.x86_64: W: no-documentation
cp2k-smp.x86_64: W: no-manual-page-for-binary cp2k.ssmp
5 packages and 0 specfiles checked; 0 errors, 6 warnings.



MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK

MUST: The spec file for the package is legible and macros are used consistently. OK
- Although I would prefer if Patch0 was split in pieces.

MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. OK
- Source code has no license headers, but GPLv2+ license is specified clearly in COPYRIGHT and src/cp2k_info.F

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
e46efdeb7230cb762245619a1d275e03  cp2k-2_1-branch.tar.gz
e46efdeb7230cb762245619a1d275e03  ../SOURCES/cp2k-2_1-branch.tar.gz

MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK

MUST: Large documentation files must go in a -doc subpackage. OK
- doc directory contains some large PDFs, but they are not very relevant to the operation of the package and are not shipped.

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
SHOULD: %{?dist} tag is used in release. OK

SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. NEEDSWORK

SHOULD: The package builds in mock. OK
EPEL: Clean section exists. OK
EPEL: Buildroot cleaned before install. OK
EPEL: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A

Comment 4 Susi Lehtola 2010-12-06 14:32:11 UTC
Also, you should note that I recompiled libint to support higher values of angular momentum a week ago. The update should hit the stable repo as soon as all the dependent packages have been rebuilt.

To account for this you will need to pass "-D__LIBINT_MAX_AM=7 -D__LIBDERIV_MAX_AM1=6" in the compilation as instructed in tools/hfx_tools/libint_tools/README_LIBINT.

Comment 5 Dominik 'Rathann' Mierzejewski 2010-12-06 23:33:12 UTC
Spec URL: http://rathann.fedorapeople.org/review/cp2k.spec
SRPM URL:
http://rathann.fedorapeople.org/review/cp2k-2.1-2.20101006.fc13.src.rpm

All done. Why did you write:

> SHOULD: If the package does not include license text(s) as separate files from
> upstream, the packager should query upstream to include it. NEEDSWORK

?

Comment 6 Susi Lehtola 2010-12-07 01:59:21 UTC
(In reply to comment #5)
> All done. Why did you write:
> 
> > SHOULD: If the package does not include license text(s) as separate files from
> > upstream, the packager should query upstream to include it. NEEDSWORK
> 
> ?

Because of the item in http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
and because the GPL license is not included (only the COPYRIGHT) which is a different thing.

Okay, compilation is now against ATLAS and libint support seems to be OK.
I haven't run a test calculation, however; maybe I'll get the time to do that in the near future.

The package has been

APPROVED

Comment 7 Dominik 'Rathann' Mierzejewski 2010-12-09 13:50:56 UTC
Thanks a lot!

New Package SCM Request
=======================
Package Name: cp2k
Short Description: A molecular dynamics engine capable of classical and Car-Parrinello simulations
Owners: rathann
Branches: f13 f14 el6
InitialCC:

Comment 8 Jason Tibbitts 2010-12-10 14:44:40 UTC
Git done (by process-git-requests).

Comment 9 Dominik 'Rathann' Mierzejewski 2010-12-12 03:35:46 UTC
Jussi, could you build libint on EPEL-6 (and maybe EPEL-5, too)? It's missing currently.

Comment 10 Fedora Update System 2010-12-13 21:27:27 UTC
cp2k-2.1-2.20101006.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/cp2k-2.1-2.20101006.fc13

Comment 11 Fedora Update System 2010-12-13 21:27:34 UTC
cp2k-2.1-2.20101006.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/cp2k-2.1-2.20101006.fc14

Comment 12 Fedora Update System 2010-12-15 08:59:50 UTC
cp2k-2.1-2.20101006.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update cp2k'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/cp2k-2.1-2.20101006.fc14

Comment 13 Fedora Update System 2010-12-26 19:53:27 UTC
cp2k-2.1-2.20101006.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 14 Fedora Update System 2010-12-26 19:54:06 UTC
cp2k-2.1-2.20101006.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Dominik 'Rathann' Mierzejewski 2011-03-04 17:22:12 UTC
Package Change Request
=======================
Package Name: cp2k
New Branches: el5
Owners: rathann

Comment 16 Jason Tibbitts 2011-03-04 18:48:28 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.