GNU bug report logs - #64504
[PATCH] gnu: Add unsio.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64504 in the body.
You can then email your comments to 64504 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sharlatan Hellseher <sharlatanus <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Sharlatan Hellseher <sharlatanus <at> gmail.com>,
 Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
Subject: [PATCH] gnu: Add unsio.
Date: Thu,  6 Jul 2023 23:13:05 +0100

* gnu/packages/astronomy.scm (unsio): New variable.

Co-Authored-By: Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
---
 gnu/packages/astronomy.scm | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 3a1b584808..15590cb84b 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2021, 2022 Vinicius Monego <monego <at> posteo.net>
 ;;; Copyright © 2021 Greg Hogan <code <at> greghogan.com>
 ;;; Copyright © 2021 Foo Chuan Wei <chuanwei.foo <at> hotmail.com>
+;;; Copyright © 2023 Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -74,6 +75,7 @@ (define-module (gnu packages astronomy)
   #:use-module (gnu packages python-xyz)
   #:use-module (gnu packages qt)
   #:use-module (gnu packages readline)
+  #:use-module (gnu packages sqlite)
   #:use-module (gnu packages sphinx)
   #:use-module (gnu packages textutils)
   #:use-module (gnu packages time)
@@ -3767,3 +3769,39 @@ (define-public python-wiimatch
 for optimal @code{matching} of weighted N-dimensional image intensity data
 using (multivariate) polynomials.")
     (license license:bsd-3)))
+
+(define-public unsio
+  ;; There is no versioned tag, use the latest commit.
+  (let ((commit "25e52468298e1194c9726ef5dba9d5fbb46870f5")
+        (revision "0"))
+    (package
+      (name "unsio")
+      (version (git-version "1.3.3" revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://gitlab.lam.fr/infrastructure/unsio")
+                      (commit commit)))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "110i2p5608zhh5w3pf3b5r2651hykw2ayspgq6vpqsffhya1p170"))))
+      (build-system cmake-build-system)
+      (arguments
+       (list #:tests? #f ;No tests
+             #:build-type "Release"
+             #:configure-flags #~(list "-DCMAKE_CXX_STANDARD=14")))
+      (inputs (list gfortran hdf5 perl sqlite zlib))
+      (home-page "https://projets.lam.fr/projects/unsio/wiki")
+      (synopsis "Universal Nbody snapshot I/O")
+      (description
+       "@acronym{UNSIO, Universal Nbody Snapshot Input Output} is an API which
+perform input/output operations in a generic way, and on different kind of nbody
+files format (nemo, Gadget binaries 1 and 2, Gadget hdf5, Ramses).  By using this
+API, a user could write only one analysis program which will work on all known
+files format supported by UNSIO. It's not necessary anymore to know how is
+implemented a file format, UNSIO will do transparently and automatically all the
+hard work for you!  With UNSIO, you will spend less time to develop your
+analysis program.  UNSIO comes with an integrated sqlite3 database which can be
+used to retrieve automatically all your data among terabytes of hard disks.")
+      (license license:cecill))))

base-commit: de3b8684e9a8e90e243cc2061100b06576c04077
-- 
2.40.1

Message #8 received at 64504 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Enge <andreas <at> enge.fr>
To: Sharlatan Hellseher <sharlatanus <at> gmail.com>
Cc: 64504 <at> debbugs.gnu.org, Efraim Flashner <efraim <at> flashner.co.il>,
 Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#64504] [PATCH] gnu: Add unsio.
Date: Tue, 8 Aug 2023 11:29:34 +0200

[Message part 1 (text/plain, inline)]

Hello,

thanks for the patch! I tried to remove the marketing speech out of the
description. For the #:build-type, the default value (release with debug
information) also works and results in a package of the same size. Is there
a reason to change it? Modified patch attached.

I can also compile the package without the configure flags, are they
useful?

In the end dynamic and static libraries end up in a directory lib64/.
Should this not be lib/?

And there are lots of warnings about 34 bit shifts in a 32 bit type.
But I suppose you tested that the library works.

Andreas

[0001-gnu-Add-unsio.patch (text/plain, attachment)]

Message #11 received at 64504 <at> debbugs.gnu.org (full text, mbox):

From: Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
To: Andreas Enge <andreas <at> enge.fr>
Cc: 64504 <at> debbugs.gnu.org, Sharlatan Hellseher <sharlatanus <at> gmail.com>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#64504] [PATCH] gnu: Add unsio.
Date: Tue, 8 Aug 2023 13:37:14 +0200

Hello!

I am the author of the initial version of this patch (kudos to Sharlatan 
for cleaning it up and bringing to guix proper!)

On 8/8/23 11:29, Andreas Enge wrote:
> For the #:build-type, the default value (release with debug information) also works and results in a package of the same size. Is there a reason to change it? Modified patch attached.
The debug build type is unnecessary verbose (see src/uns.cc line 71-73 
for an example) to the point that it impacts performance sometimes, 
which is important considering the usecase of the library.

> I can also compile the package without the configure flags, are they useful?
Checked again, I still cannot compile it without -DCMAKE_CXX_STANDARD=14


> And there are lots of warnings about 34 bit shifts in a 32 bit type.
> But I suppose you tested that the library works.
I have not noticed any issues using it during the past two years, but 
this cannot be considered a proper test because I am actually using a 
small subset of the library. Perhaps, this warning should be reported 
upstream, thanks!


All the best,
Iliya

Message #14 received at 64504 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Enge <andreas <at> enge.fr>
To: Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
Cc: 64504 <at> debbugs.gnu.org, Sharlatan Hellseher <sharlatanus <at> gmail.com>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#64504] [PATCH] gnu: Add unsio.
Date: Tue, 8 Aug 2023 17:49:50 +0200

[Message part 1 (text/plain, inline)]

Am Tue, Aug 08, 2023 at 01:37:14PM +0200 schrieb Iliya Tikhonenko:
> On 8/8/23 11:29, Andreas Enge wrote:
> > For the #:build-type, the default value (release with debug information) also works and results in a package of the same size. Is there a reason to change it? Modified patch attached.
> The debug build type is unnecessary verbose (see src/uns.cc line 71-73 for
> an example) to the point that it impacts performance sometimes, which is
> important considering the usecase of the library.

Okay, I have reinstated your line and added a comment.
 
> > I can also compile the package without the configure flags, are they useful?
> Checked again, I still cannot compile it without -DCMAKE_CXX_STANDARD=14

I can... Patch attached.

> > And there are lots of warnings about 34 bit shifts in a 32 bit type.
> > But I suppose you tested that the library works.
> I have not noticed any issues using it during the past two years, but this
> cannot be considered a proper test because I am actually using a small
> subset of the library. Perhaps, this warning should be reported upstream,
> thanks!

/tmp/guix-build-unsio-1.3.3-0.25e5246.drv-0/source/src/snapshotinterface.cc:125:45: warning: result of ‘(-2147483648 << 2)’ requires 34 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=]
  125 |       req_bits = (unsigned int) (( 1 << 31 )<<2)-1;
      |                                 ~~~~~~~~~~~~^~~~

Here is the offending line. This is indeed a bit surprising, as "int"
usually has 32 bits. The 1 to be shifted is a signed int; 1*2^31 is not
representable in the type, so if I understand the C standard correctly,
the result is undefined. Then I have seen a Stackoverflow comment that
it is okay in C++20. And indeed:
   #:configure-flags #~(list "-DCMAKE_CXX_STANDARD=20")
passes without these warnings. Should we add this?
Although such constructs look a bit brittle to me, and it is not
suggested by the author.

There is still the question about the lib64/ installation directory,
I think this should be changed.

Andreas

[0001-gnu-Add-unsio.patch (text/plain, attachment)]

Message #17 received at 64504 <at> debbugs.gnu.org (full text, mbox):

From: Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
To: Andreas Enge <andreas <at> enge.fr>
Cc: 64504 <at> debbugs.gnu.org, Sharlatan Hellseher <sharlatanus <at> gmail.com>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#64504] [PATCH] gnu: Add unsio.
Date: Mon, 14 Aug 2023 14:41:55 +0200

On 8/8/23 17:49, Andreas Enge wrote:
> Am Tue, Aug 08, 2023 at 01:37:14PM +0200 schrieb Iliya Tikhonenko:
>> On 8/8/23 11:29, Andreas Enge wrote:
>> The debug build type is unnecessary verbose (see src/uns.cc line 71-73 for
>> an example) to the point that it impacts performance sometimes, which is
>> important considering the usecase of the library.
> Okay, I have reinstated your line and added a comment.Fine, maybe "to disable debug printing" would sound slightly more clear, 
but I am not sure.

>> Checked again, I still cannot compile it without -DCMAKE_CXX_STANDARD=14
> I can... Patch attached.
Yeah, it builds now.
>     #:configure-flags #~(list "-DCMAKE_CXX_STANDARD=20")
> passes without these warnings. Should we add this?
> Although such constructs look a bit brittle to me, and it is not suggested by the author.
> 
> There is still the question about the lib64/ installation directory,
> I think this should be changed.

I think I am fine with both changes. I did a few quick tests and have 
not noticed any issues.


All the best,
Iliya

Message #22 received at 64504-done <at> debbugs.gnu.org (full text, mbox):

From: Andreas Enge <andreas <at> enge.fr>
To: Iliya Tikhonenko <tikhonenko <at> mpe.mpg.de>
Cc: 64504-done <at> debbugs.gnu.org, Sharlatan Hellseher <sharlatanus <at> gmail.com>,
 Efraim Flashner <efraim <at> flashner.co.il>, Eric Bavier <bavier <at> posteo.net>
Subject: Re: [bug#64504] [PATCH] gnu: Add unsio.
Date: Thu, 17 Aug 2023 20:10:03 +0200

Hello,

On 8/8/23 17:49, Andreas Enge wrote:
> There is still the question about the lib64/ installation directory,
> I think this should be changed.

I just did with a little snippet; this has been programmed manually by the
author, so could not be changed using cmake variables.

Pushed, thanks for the new package!

Andreas

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.