sailfish-team / sailfish

Lattice Boltzmann (LBM) simulation package for GPUs (CUDA, OpenCL)
http://sailfish.us.edu.pl
233 stars 85 forks source link

Multigpu #3

Closed kasiaj closed 12 years ago

kasiaj commented 12 years ago

Documentation and regression test

mjanusz commented 12 years ago

Good job on the automated regression test! The code mostly looks good, but there are a bunch of style fixes that need to be applied to keep the code consistent with the rest of project. We're following the standard Python style conventions. You might find tools such as pyflakes or pylint helpful in finding and fixing simple style issues (just run the tool on the .py file and fix any warnings that you think make sense).

I'll take a look at the documentation changes later today. Thanks!

mjanusz commented 12 years ago

One final comment about the regtest file -- could you please make sure that are there are never more than 2 empty consecutive lines, and also that functions/classes don't start with an empty line?

mjanusz commented 12 years ago

Thanks for addressing the previous comments. I added a bunch of comments about minor style issues. I think we're very close to getting this merged :)

mjanusz commented 12 years ago

Thanks for the fixes! I added a final set of minor style comments. Other than that the change look good, thanks for your work on this!

Before we can merge this, we need to clean up the history a little bit, as all commits from your branch will be copied to the main Sailfish repository. I propose merging most of the corrections into the first few commits. You can do so by rewriting the history in your repository as follows:

git rebase -i HEAD~20 (edit the file by changing the lines with correction commits so that they start with 'f'; please read the in-file comments to see what that means) (close the editor and let git rewrite the local history) git push --force

Before you do that, make a local backup of the repository just in case something goes wrong.

mjanusz commented 12 years ago

Thanks for your work on this -- I've just merged your code into the multigpu branch.