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

Review Request: Stollmeier #27

Closed
wants to merge 6 commits into
base: master
from

Conversation

@Fjanks

Fjanks commented Feb 22, 2017

AUTHOR

Dear @ReScience/editors,

I request a review for the following replication:

Original article

Title: A simple rule for the evolution of cooperation on graphs and social networks
Author(s): H. Ohtsuki, C. Hauert, E. Lieberman and M.A. Nowak
Journal (or Conference): Nature
Year: 2006
DOI: 10.1038/nature04605
PDF: http://projects.iq.harvard.edu/files/ped/files/nature06a_0.pdf (SI: http://www.nature.com/nature/journal/v441/n7092/extref/nature04605-s1.pdf)

Replication

Author(s): F. Stollmeier (ORCID: 0000-0003-4858-0895)
Repository: https://github.com/Fjanks/ReScience-submission/tree/STOLLMEIER-2017
PDF: https://github.com/Fjanks/ReScience-submission/blob/STOLLMEIER-2017/article/stollmeier-2017.pdf
Keywords: Evolutionary Game Theory, Networks
Language: English
Domain:

Results

  • Article has been fully replicated
  • Article has been partially replicated
  • Article has not been replicated

EDITOR

  • Editor acknowledgement (@ctb) February 24, 2017
  • Reviewer 1 (@AdamRTomkins) February 27, 2017
  • Reviewer 2 (@anyaevostinar) February 28, 2017
  • Reviewer 3 (@KamilSJaron) June 15, 2017
  • Review 1 decision [accept] April 19, 2017
  • Review 2 decision [accept] July 1, 2017
  • Editor decision [accept] July 4, 2017
@rougier

This comment has been minimized.

Member

rougier commented Feb 23, 2017

Thanks for your submission, we'll assign an editor soon and will proceed with review.
Don't hesitate to ask for update.

@rougier

This comment has been minimized.

Member

rougier commented Feb 23, 2017

@Fjanks By the way, do you have a link to a free PDF of the original article ?

@rougier

This comment has been minimized.

Member

rougier commented Feb 23, 2017

@ctb Could you edit this submission ?

@rougier

This comment has been minimized.

Member

rougier commented Feb 23, 2017

@Fjanks And I forgot, can you add your ORCID id ? (you should be able to update your first post)

@Fjanks

This comment has been minimized.

Fjanks commented Feb 24, 2017

@rougier
I replaced the link to the original article with a link to a free pdf and also added a link to the supplementary.
I put the ORCID id after my name in the first post. Is that ok or is there somewhere a field for the ORCID id which I didn't see?

@ctb

This comment has been minimized.

ctb commented Feb 24, 2017

@rougier I can edit!

@AdamRTomkins

This comment has been minimized.

AdamRTomkins commented Feb 24, 2017

I would happily act as a reviewer for this submission.

@rougier

This comment has been minimized.

Member

rougier commented Feb 24, 2017

@Fjanks perfect !

@rougier

This comment has been minimized.

Member

rougier commented Feb 24, 2017

@AdamRTomkins thanks !

@ctb

This comment has been minimized.

ctb commented Feb 27, 2017

@AdamRTomkins please go ahead! I'm looking for another reviewer now, but there's no reason not to start :).

For your reference, here are reviewer guidelines: https://github.com/ReScience/ReScience/blob/master/reviewer-guidelines.md

@ctb

This comment has been minimized.

ctb commented Feb 28, 2017

@anyaevostinar has also agreed to review! @rougier do you need to do anything to give her perms? her ORCID is orcid.org/0000-0001-7216-5283 - thanks, Anya!

@rougier

This comment has been minimized.

Member

rougier commented Mar 1, 2017

@ctb @anyaevostinar Simplest would be to register as reviewer here

@ctb Don't forget to update PR labels and information at the start of the htread (reviewer names and date)

@anyaevostinar

This comment has been minimized.

anyaevostinar commented Mar 1, 2017

@ctb @rougier registered

@rougier rougier added the 02 - Review label Mar 1, 2017

@ctb ctb removed the 01 - Request label Mar 2, 2017

@anyaevostinar

This comment has been minimized.

anyaevostinar commented Mar 9, 2017

I'm having some problems getting "Preparations" to run. I assumed I should enter: jupyter notebook on the command line and then click replication.ipynb and then start executing. I've installed all the packages it looks like I need, but am getting:

---------------------------------------------------------------------------
DistutilsExecError                        Traceback (most recent call last)
/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/unixccompiler.py in _compile(self, obj, src, ext, cc_args, extra_postargs, pp_opts)
    115             self.spawn(compiler_so + cc_args + [src, '-o', obj] +
--> 116                        extra_postargs)
    117         except DistutilsExecError as msg:

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/ccompiler.py in spawn(self, cmd)
    908     def spawn(self, cmd):
--> 909         spawn(cmd, dry_run=self.dry_run)
    910 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/spawn.py in spawn(cmd, search_path, verbose, dry_run)
     35     if os.name == 'posix':
---> 36         _spawn_posix(cmd, search_path, dry_run=dry_run)
     37     elif os.name == 'nt':

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/spawn.py in _spawn_posix(cmd, search_path, verbose, dry_run)
    161                           "command %r failed with exit status %d"
--> 162                           % (cmd, exit_status))
    163             elif os.WIFSTOPPED(status):

DistutilsExecError: command 'gcc-4.2' failed with exit status 1
@Fjanks

This comment has been minimized.

Fjanks commented Mar 10, 2017

That looks like the compilation of the cython code fails. Did you check whether cython in general works? You could test it with the examples on http://docs.cython.org/en/latest/src/quickstart/build.html.

By the way, I saw in the error message that you use python 3. That reminded me that I totally forgot to make the code compatible with python 3, which I did some minutes ago.

@anyaevostinar

This comment has been minimized.

anyaevostinar commented Mar 17, 2017

@Fjanks I have gone through the hello world example and the jupyter example in the cython docs and that all works as expected. I've tried again with Python3 and am getting the same:

---------------------------------------------------------------------------
DistutilsExecError                        Traceback (most recent call last)
/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/unixccompiler.py in _compile(self, obj, src, ext, cc_args, extra_postargs, pp_opts)
    115             self.spawn(compiler_so + cc_args + [src, '-o', obj] +
--> 116                        extra_postargs)
    117         except DistutilsExecError as msg:

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/ccompiler.py in spawn(self, cmd)
    908     def spawn(self, cmd):
--> 909         spawn(cmd, dry_run=self.dry_run)
    910 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/spawn.py in spawn(cmd, search_path, verbose, dry_run)
     35     if os.name == 'posix':
---> 36         _spawn_posix(cmd, search_path, dry_run=dry_run)
     37     elif os.name == 'nt':

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/spawn.py in _spawn_posix(cmd, search_path, verbose, dry_run)
    161                           "command %r failed with exit status %d"
--> 162                           % (cmd, exit_status))
    163             elif os.WIFSTOPPED(status):

DistutilsExecError: command 'gcc-4.2' failed with exit status 1

During handling of the above exception, another exception occurred:

CompileError                              Traceback (most recent call last)
/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/site-packages/pyximport/pyximport.py in load_module(name, pyxfilename, pyxbuild_dir, is_package, build_inplace, language_level, so_path)
    215             so_path = build_module(module_name, pyxfilename, pyxbuild_dir,
--> 216                                    inplace=build_inplace, language_level=language_level)
    217         mod = imp.load_dynamic(name, so_path)

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/site-packages/pyximport/pyximport.py in build_module(name, pyxfilename, pyxbuild_dir, inplace, language_level)
    191                                   inplace=inplace,
--> 192                                   reload_support=pyxargs.reload_support)
    193     assert os.path.exists(so_path), "Cannot find: %s" % so_path

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/site-packages/pyximport/pyxbuild.py in pyx_to_dll(filename, ext, force_rebuild, build_in_temp, pyxbuild_dir, setup_args, reload_support, inplace)
    101         obj_build_ext = dist.get_command_obj("build_ext")
--> 102         dist.run_commands()
    103         so_path = obj_build_ext.get_outputs()[0]

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/dist.py in run_commands(self)
    954         for cmd in self.commands:
--> 955             self.run_command(cmd)
    956 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/dist.py in run_command(self, command)
    973         cmd_obj.ensure_finalized()
--> 974         cmd_obj.run()
    975         self.have_run[command] = 1

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/site-packages/Cython/Distutils/old_build_ext.py in run(self)
    184 
--> 185         _build_ext.build_ext.run(self)
    186 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/command/build_ext.py in run(self)
    338         # Now actually compile and link everything.
--> 339         self.build_extensions()
    340 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/site-packages/Cython/Distutils/old_build_ext.py in build_extensions(self)
    192             ext.sources = self.cython_sources(ext.sources, ext)
--> 193             self.build_extension(ext)
    194 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/command/build_ext.py in build_extension(self, ext)
    502                                          extra_postargs=extra_args,
--> 503                                          depends=ext.depends)
    504 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/ccompiler.py in compile(self, sources, output_dir, macros, include_dirs, debug, extra_preargs, extra_postargs, depends)
    573                 continue
--> 574             self._compile(obj, src, ext, cc_args, extra_postargs, pp_opts)
    575 

/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/distutils/unixccompiler.py in _compile(self, obj, src, ext, cc_args, extra_postargs, pp_opts)
    117         except DistutilsExecError as msg:
--> 118             raise CompileError(msg)
    119 

CompileError: command 'gcc-4.2' failed with exit status 1
@rougier

This comment has been minimized.

Member

rougier commented Mar 17, 2017

@anyaevostinar My 2 cents but do you have gcc-4.2 installed ?

@Fjanks

This comment has been minimized.

Fjanks commented Mar 20, 2017

If the examples in the cython docs work then I guess that pyximport
is the problem. To test this you could use the hello.pyx from the examples and try to import it in python with

import pyximport; pyximport.install()
import hello

If that fails maybe it helps to reinstall pyximport. As an alternative, you can do it completely without pyximport, the algorithms.pyx can also be compiled with distutils in
the same way as described for the hello.pyx in the docs. Just replace the
filename 'hello.pyx' by 'algorithms.pyx' in the setup.py script. If that works
remove the line "import pyximport; pyximport.install()" in the
jupyter notebook.

@anyaevostinar

This comment has been minimized.

anyaevostinar commented Mar 20, 2017

@rougier Yes, though here is the full version info:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1 Apple LLVM version 7.0.2 (clang-700.1.81) Target: x86_64-apple-darwin14.5.0 Thread model: posix

@Fjanks That pyximport code works correctly with only 'unused function' warnings. I'll try without pyximport anyway.

@Fjanks

This comment has been minimized.

Fjanks commented Jun 23, 2017

Thanks for pointing at that part of the code! I think you found the problem.
If I use always the same seed, e.g. seeds = [1 for i in xrange(n_graphs)], I get results that look very much like yours.
I have to read a bit to understand how to generate random number more reliably.

@KamilSJaron

This comment has been minimized.

KamilSJaron commented Jun 23, 2017

Just to add. The code looks really good and it is very easy to tweak it, so I tried to simulate complete graphs of 2, 4 and 8 individuals with no cost and no benefit resulting in expected probabilities of fixation 0.5, 0.25, 0.125. It seems that this basic behaviour is correct.

Now I am going to wait till @Fjanks commit the change of random number generation.

@dimpase

This comment has been minimized.

dimpase commented Jun 23, 2017

@Fjanks

This comment has been minimized.

Fjanks commented Jun 23, 2017

@dimpase: Yes, the random graphs generated by create_connected_random_graph(N,k) are biased. I decided to use this algorithm because that is how I understand the description of the random graph generation in the supplementary of the original paper. And if I understood it correctly (their description is not very detailed) only results from this method are comparable to the results in the original paper.

@Fjanks

This comment has been minimized.

Fjanks commented Jun 27, 2017

Ok, I did a bit research on the issue with rand(). As KamilSJaron already mentioned, it is well known that random numbers from rand() have low quality, so I should not have chosen it in the first place. However, a small period, autocorrelation or a non-uniform distribution do not explain KamilSJaron's results, and for the effect I assume to be the root of the problem I did not find a reference. So I made a little test: On my system the first number from rand() varies with different seeds, but on the Mac of a colleague the first number from rand() was the same for different seed values. This effect results in one node of the network being more often chosen as the position of the initial cooperator than the other nodes. Consequently, the average values for the fixation probabilities would require much larger numbers of simulations to approach the expected values.
To avoid these problems I replaced rand() by Mersenne Twister. Also for Mersenne Twister it seems not so trivial to choose a good set of seed values, but it is not platform dependent and I checked that the first few numbers across the seed values are to a good approximation uniformly distributed.

I also added the setup.py and updated the compilation instructions.

@KamilSJaron

This comment has been minimized.

KamilSJaron commented Jun 30, 2017

It seems that it was indeed problem of non-random seeding. The simulations look way better now. So far I simulated only few replicates of the fixation probabilities for different cost values and the graph already looks very similar to the one presented in paper (with more noise of course, but it seems that it quickly converge to the presented fixation probabilities).

Concerning random numbers, I will just have a comment. You have seeded the generator with hardcoded seed values for every possible value of n_graphs.

seeds = np.linspace(1, 2**32-1, n_graphs, dtype=np.uint32)

The good practise is to allow a seed specified in an argument of the simulator's wrapper and if it is not specified you can generate it using time (not optimal either) or non-seedable generators (like random_device). However, the current solution is totally fine for reproduction of the results from the original paper.

I will get back once I will re-simulate at least couple of the different graphs.

@KamilSJaron

This comment has been minimized.

KamilSJaron commented Jul 1, 2017

I managed to reproduce results of @Fjanks (the simulations of 100 individuals). Therefore I believe that @Fjanks successfully reproduced simulations presented in the original paper "A reference implementation of A simple rule for the evolution of cooperation on graphs and social networks.".

@ctb I suggest to accept the manuscript. My objections were incorporated already.

@ctb

This comment has been minimized.

ctb commented Jul 4, 2017

Thanks all! it looks like the new decision should be accept based on #27 (comment) - @anyaevostinar any further thoughts? If not, then I will click the magic "accept" button :)

@rougier

This comment has been minimized.

Member

rougier commented Jul 5, 2017

@ctb Don't forget to update the post at the top with relevant dates (you'll need them later)

@Fjanks

This comment has been minimized.

Fjanks commented Jul 13, 2017

Today, while working on another project where I use parts of this code, I discovered a bug in the function create_connected_random_graph(). As a consequence, the average degree of the connected random graphs was slightly smaller than expected.
I fixed the bug and updated the figures in the article. The differences in the three affected plots are almost invisible. Just in case someone would run the simulations with large average degrees it could make a significant difference.

@ctb ctb added 03 - Accepted and removed 02 - Review labels Jul 18, 2017

@ctb

This comment has been minimized.

ctb commented Jul 18, 2017

Accepted! Please lock this PR @rougier as I do not have permissions?

@ctb ctb closed this Jul 18, 2017

@ctb

This comment has been minimized.

@ReScience ReScience locked and limited conversation to collaborators Jul 18, 2017

@rougier

This comment has been minimized.

Member

rougier commented Jul 18, 2017

I locked it but I think you have permission.

@rougier

This comment has been minimized.

Member

rougier commented Aug 10, 2017

@ctb Can you handle the publication?

@rougier rougier requested a review from anyaevostinar Aug 10, 2017

@rougier rougier reopened this Aug 10, 2017

@rougier

This comment has been minimized.

Member

rougier commented Aug 22, 2017

@khinsen Can you handle the publication?

@khinsen

This comment has been minimized.

khinsen commented Aug 23, 2017

@rougier This article is accepted, so "handling" just means the technicalities of publication, right? In that case, yes, I can do that.

@rougier

This comment has been minimized.

Member

rougier commented Aug 23, 2017

Yes, I think @ctb forgot to do it. It might be easier if we do it now. I'll have more time by next week so I can do it either if your prefer.

@khinsen

This comment has been minimized.

khinsen commented Aug 23, 2017

EDITOR

This submission has been published and will soon appear at http://rescience.github.io/read/

DOI

@khinsen khinsen closed this Aug 23, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.