ReScience / submissions

ReScience C submissions
28 stars 7 forks source link

[Rp] Gifa V.4: A complete package for NMR data set processing #33

Closed delsuc closed 4 years ago

delsuc commented 4 years ago

Original article: [Rp] Gifa V.4: A complete package for NMR data set processing

PDF URL: https://github.com/delsuc/Gifa/blob/master/article/article.pdf Metadata URL: https://github.com/delsuc/Gifa/blob/master/article/metadata.yaml Code URL: https://github.com/delsuc/Gifa/blob/master/code

Scientific domain: NMR, data processing Programming language: Fortran77, C, Gifa macro Suggested editor: Pierre de Buyl, Konrad Hinsen, Thomas Arildsen, Georgios Detorakis

rougier commented 4 years ago

@pdebuyl @khinsen Can you serve as editor for this submission for the Ten Years Reproducibility Challenge ?

rougier commented 4 years ago

@delsuc Thanks for your submission, we'll assign an editor soon.

pdebuyl commented 4 years ago

I can edit. @khinsen can you review?

khinsen commented 4 years ago

@pdebuyl Yes, I will do a review.

khinsen commented 4 years ago

@delsuc Could you add instructions for compiling, installing, and running the tests? The code archive looks like this should be straightforward, but it isn't obvious how to start. Also, the paper suggests that running the code requires a 32-bit system. That should also be stated in the README so that people don't waste time trying anything else.

delsuc commented 4 years ago

You're right, I should find some time today to do this.

delsuc commented 4 years ago

Thanks @khinsen , that was needed, and I should have done it in the first hand. I added a part in the README which shows how to build the binary, and reproduce the figures in the manuscript.

khinsen commented 4 years ago

Thanks @delsuc, that helps a lot!

Following the instructions, I almost managed to compile Gifa. I had to install the package libxext-dev in order to succeed. Then I could run the tests.

I haven't been able so far to reproduce the figures. Here's what I do: 1) cd Gifa/article 2) gifa (this opens a few windows) 3) fig2a.g (this open a dialog box entitled "Display control") 4) Push the OK button.

I get the promised error message about dispcont_doit, but no figure. All windows remain blank.

Next: 1) cd Gifa/article 2) gifa (this opens a few windows) 3) figure3.g (this open a dialog box entitled "figure_3", asking me to close the 'gifa' bar and then the dialog) 4) I close the 'gifa' bar (by closing the window) 5) I close the dialog via the OK button. 6) Error message in the terminal: 'No such file or directory. Aborting execution of command file : figure3.g At line : 11 Error in processing command : READ'

It looks like the macro loads a data file under Gifa/data, so I tried again from my home directory (which contains Gifa), and then I do get figure 3.

BTW, it would be useful to tell adventurous explorers that "exit" is the command to get out of Gifa!

delsuc commented 4 years ago

Thank you @khinsen for pointing out these errors !

khinsen commented 4 years ago

Thanks @delsuc, now everything works satisfactorily for me. So here comes my review! (pinging @pdebuyl !)

I enjoyed reading this article, which describes the resurrection of a piece of software from the late 1980s/early 1990s that was developed using the top state of the art of that time, paying careful attention to both efficiency and usability. What this reproduction illustrates is (1) the stability of the technology of the time, (2) in particular for software that was from the start written to be portable between the many Unix flavors of the day. The only critical aspect from a long-term reproducibility point of view is the reliance of the memory management architecture on a 32-bit address space.

There is one minor mistake in the paper: there is no "GNU foundation". That should probably be the GNU Project, which is supported by the Free Software Foundation.

The paper is readable as-is but could benefit from a round of careful proofreading for typos etc.

delsuc commented 4 years ago

Thank you @khinsen for this nice review and for helping in improving the quality of the software as well as the manuscript. I will correct the GNU point, and will go carefully through the text for proof reading.

pdebuyl commented 4 years ago

Thank you @khinsen for the review!

@delsuc I will wait for an update from your side.

delsuc commented 4 years ago

@pdebuyl I did my best to correct typos and improve the text. It is now available on the repository

I have two remarks:

khinsen commented 4 years ago

@delsuc Please see https://github.com/delsuc/Gifa/pull/1 for the small corrections required to compile the article.

As for the archive, you only need to request SWH to archive your repository, no registration required. The request form is at https://archive.softwareheritage.org/save/. Just paste the git reference for your repository into the field at the bottom and push "submit". It can take a few hours before your request is processed. You can check progress on the tab "Browse save requests". You don't need to deposit anything on Zenodo, that's the editors' job.

pdebuyl commented 4 years ago

@delsuc thank you for the update. I'll go over it "soon" :-)

pdebuyl commented 4 years ago

Hi @delsuc ,

A few minor comments:

  1. I don't know the expression "a carrier long effort" (in "Introduction"), I would think that it is not commonly used in English.
  2. There is a bit of chance in having a compatibility mode from "gdbm" for "ndbm", maybe this could be stated in the conclusion.
  3. Question: is there any code taken from the Numerical Recipes?

Very interesting read! The abandonment of the java version over the Fortran/C one is speaking!

Out of curiosity, is gifa in use in current research projects?

I will proceed to the publication this week. Comment 1 is optional (I am not a native speaker myself), as well as comment 2.

delsuc commented 4 years ago

Hi @pdebuyl , Thank you for the comments. I reply here to each points

  1. this is a silly error of mine - the correct spelling is of course "a career-long effort"
  2. a sentence has been added in the conclusion (and some minor corrections as well)
  3. Numerical Recipes (N.R.) has been used in several places
    • in the Fourier transform codes (Cpx and Real FFT and inverse FFT) - however the logic of the algorithm was modified to adapt causality in Maximum Entropy optimisation
    • in the powell optimiser, but the code was slightly adapted
    • in the random number generator - unmodified it seems
    • then Brent of Levenberg-Marquardt methods, cited in the text are inspired from N.R. but rewritten to my "hand"

The text has been modified following points 1 and 2.

pdebuyl commented 4 years ago

Thank you @delsuc for the reply and updates. This is my first paper with the new editorial scripts for ReScience, I'll get to it.

pdebuyl commented 4 years ago

@ReScience/editors / @ReScience/reviewers are people around here able to access zenodo's sandbox? I can't login since three days and would like to proceed with the publication.

otizonaizit commented 4 years ago

The zenodo sandbox was not working for me either last week as I published two ReScience papers. I just crossed my fingers and went directly for the real thing…

pdebuyl commented 4 years ago

Thanks @otizonaizit I'll it the same way :-)

pdebuyl commented 4 years ago

Thank you @delsuc for the merges. The file is online on zenodo https://zenodo.org/record/3904595#.XvM5xHVfhhE

@rougier I filed a PR to the website for adding the bib entry.

Thanks @khinsen for the review :-)