[Debichem-devel] RFS: libchemps2/1.4-1 -- spin-adapted DMRG for ab initio quantum chemistry

[Michael Banck]

Hi Sebastian,

On Thu, May 14, 2015 at 04:39:55PM -0400, Sebastian Wouters wrote:
> Hi Michael,
> 
> I have now also added the python package: python-chemps2:
> http://anonscm.debian.org/cgit/debichem/packages/libchemps2.git/
> 
> Is there anything else I can do?

I had a further look, sorry for the delay.  My comments:

1. Is there a good reason the source package is called "libchemps2"
while the github project and release tarballs are called just "chemps2"
(modulo capitalization)?  I agree it would be awkward to switch the git
repo basename and all, but maybe better now than after the package is in
the archive?  In general, the source package name should be called
similar to the tarballs.

2. A build on Debian unstable fails when running sphinx, cause it cannot
find a 'classic' theme:


|PYTHONPATH=. http_proxy='127.0.0.1:9' sphinx-build -N -b html -c obj-x86_64-linux-gnu/sphinx/ sphinx/ build/html
|Making output directory...
|Running Sphinx v1.2.3
|loading pickled environment... not yet created
|
|Theme error:
|no theme named 'classic' found (missing theme.conf?)
|make[1]: *** [override_dh_auto_build] Error 1

That's a bit weird, but indeed I don't have a
/usr/share/sphinx/themes/classic/theme.conf

Maybe it needs to be changed to "default"?  That one is available and
changing it to that makes the documentation build fine for me.

3. The following Build-Depends is missing:

 - libpython-dev

4. The include for HDF5 in CheMPS2/include/chemps2/MyHDF5.h is not
working for me, there's no /usr/include/hdf5.h (anymore?). If I replace
that with '#include <hdf5/serial/hdf5.h>', it builds fine.

5. I was a bit surprised that there is only libchemps2.so.1 and not
libchemps2.so.1.x.y as is usual for libtool projects, but I guess that
is from CMake?

6. As a general (targetted at upstream) comment, it seems to me that
there are really a lot of files shipped in /usr/include/chemps2.  Be
aware that removing any function signature from any of those will
require you to bump the library version.  Is that really the external
API in there or are some/most meant to be internal and should not be
shipped?

7. About the description in debian/control, I would advise the following
changes.  But those are not required before upload:

 - I would remove the first sentence of the 'design philosophy'
   paragraph, as implementation details or designs are usually
   discouraged in the description.  Maybe to make it more readable, you
   could break it down into a list of bullet-points as well, introduced
   with something like "Further Features include: [...]".

 - remove the "In the future" paragraph - the description should be
   about the current state of affairs (though I agree we are sometimes
   bad at updating it when new upstream version implement new features).

 - personally I would drop the last paragraph as well.  The homepage is
   already mentioned in the Homepage: field and in upstream-metadata,
   and the additional references are also mentioned in upstream-metadata
   (though I am not sure that information is exposed elsewhere yet).
   Finally, the documentation is shipped in the libchemps2-doc package,
   or is the documentation on your website different to it?  That said,
   if you prefer to leave that paragraph in, I'm fine with it.
  
 - maybe expand 2-RDM as (I guess?) "second order reduced density
   matrix (2-RDM)" on first mention, though then again if users don't
   know what 2-RDM means they might not get a lot out of the package
   anyway.  You are expanding most of the other acronyms (like FCI,
   CASCI, CASSCF) already, though.

 - Possibly add something like "CheMPS2 is parallelized via OpenMP." at
   the end, or am I mistaken about OpenMP?


Cheers,

Michael