Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
delsuc opened this issue Apr 30, 2020 · 22 comments
Closed

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

delsuc opened this issue Apr 30, 2020 · 22 comments

Comments

@delsuc
Copy link
Member

delsuc commented Apr 30, 2020

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
Copy link
Member

rougier commented May 4, 2020

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

@rougier
Copy link
Member

rougier commented May 4, 2020

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

@pdebuyl
Copy link
Member

pdebuyl commented May 4, 2020

I can edit. @khinsen can you review?

@khinsen
Copy link

khinsen commented May 4, 2020

@pdebuyl Yes, I will do a review.

@khinsen
Copy link

khinsen commented May 11, 2020

@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
Copy link
Member Author

delsuc commented May 11, 2020

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

@delsuc
Copy link
Member Author

delsuc commented May 11, 2020

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
Copy link

khinsen commented May 12, 2020

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
Copy link
Member Author

delsuc commented May 12, 2020

Thank you @khinsen for pointing out these errors !

  • install libxext-dev - of course - I just forgot to copy it in the README.

  • fig2a.g is really badly named ! it actually contains the listing in Figure 2a of the original paper, and reproduces Figure 2c there, which is Figure 1 from the Rescience work (sorry for the strange numbering!) - I considered that there is no need to reproduce Figure 2a and 2b from the original as these are just plain listings.
    So the appearance of the dialog box is the whole purpose - I realise this is a bit strange, so I added additional resources so that the correct behaviour is reproduced as well.

  • the README has been extended with basic Gifa usage - and how to reproduce the figures.

@khinsen
Copy link

khinsen commented May 13, 2020

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
Copy link
Member Author

delsuc commented May 13, 2020

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
Copy link
Member

pdebuyl commented May 13, 2020

Thank you @khinsen for the review!

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

@delsuc
Copy link
Member Author

delsuc commented May 18, 2020

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

I have two remarks:

  • for some reason my name and affiliation do not appear in the pdf header, and the compilation always ends-up with an error. I double checked the metadata and the tex, but I could not find where is the problem.
  • in which order should I/We register to swh and Zenodo DOI .

@khinsen
Copy link

khinsen commented May 18, 2020

@delsuc Please see delsuc/Gifa#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
Copy link
Member

pdebuyl commented May 19, 2020

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

@pdebuyl
Copy link
Member

pdebuyl commented Jun 1, 2020

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
Copy link
Member Author

delsuc commented Jun 17, 2020

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
Copy link
Member

pdebuyl commented Jun 18, 2020

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
Copy link
Member

pdebuyl commented Jun 23, 2020

@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
Copy link
Member

otizonaizit commented Jun 23, 2020 via email

@pdebuyl
Copy link
Member

pdebuyl commented Jun 23, 2020

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

@pdebuyl
Copy link
Member

pdebuyl commented Jun 24, 2020

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 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants