nipy / mindboggle

Automated anatomical brain label/shape analysis software (+ website)
http://mindboggle.info
Other
146 stars 53 forks source link

Zernike refactor #42

Closed brianthelion closed 8 years ago

brianthelion commented 10 years ago

1) Legacy code should be moved into the test suite. 2) Test suite should be updated. 3) File proliferation should be reduced or eliminated. 4) Code should be PEP8 compliant. 5) Remaining MATLAB idioms should be replaced with Numpy idioms.

Anything else?

binarybottle commented 10 years ago

Those are all good. I would add documentation of the code so that it will be easier to maintain in the future.

brianthelion commented 10 years ago

Making incremental progress here, a few hours a day. Expect a push this week.

binarybottle commented 10 years ago

Great!

Cheers, @rno

On Mon, Apr 28, 2014 at 4:05 PM, brianthelion notifications@github.comwrote:

Making incremental progress here, a few hours a day. Expect a push this week.

— Reply to this email directly or view it on GitHubhttps://github.com/binarybottle/mindboggle/issues/42#issuecomment-41625205 .

brianthelion commented 10 years ago

Didn't get quite as much done as I had hoped last week. Picking it up again tomorrow and will update on progress then. Cheers!

binarybottle commented 10 years ago

Great -- Looking forward to your update!

Cheers, @rno

On Mon, May 5, 2014 at 12:54 PM, brianthelion notifications@github.comwrote:

Didn't get quite as much done as I had hoped last week. Picking it up again tomorrow and will update on progress then. Cheers!

— Reply to this email directly or view it on GitHubhttps://github.com/binarybottle/mindboggle/issues/42#issuecomment-42230861 .

brianthelion commented 10 years ago

Documentation is still to-do.

binarybottle commented 10 years ago

Good show, Brian! After you implemented Koehl's exact method, it became significantly faster. It took only 10 minutes for order 10 for two cortical hemispheres!

brianthelion commented 10 years ago

The Koehl implementation is covered in #45. There are still a few things to do to wrap this ticket, like pulling together the documentation.

brianthelion commented 9 years ago

@binarybottle: I finally have some time to put on this cleanup task. Unfortunately I haven't tracked our priorities w.r.t tasks 1-5 listed above. Is documentation the only thing that remains to be done at this time? Cheers!

binarybottle commented 9 years ago

I believe documentation was the only thing left to do, but after Satra found that a 2GB memory limit was exceeded by computing Zernike moments, this strikes me as extremely important to address!

From Satra: 150215-15:17:45,874 workflow INFO: Executing node Zernike_sulci.a1 in dir: /om/scratch/Tue/ps/MB_work/734db8e05f6be469df79c1419f253ad7/Mindboggle/Surface_feature_shapes/_hemi_rh/Zernike_sulci Load "sulci" scalars from sulci.vtk 8329 vertices for label 1 Reduced 160076 to 15921 triangular faces srun: Exceeded job memory limit

brianthelion commented 9 years ago

Ok, see my comments on #52.

brianthelion commented 9 years ago

@binarybottle Is there a particular documentation standard that you're sticking with on Mindboggle?

binarybottle commented 9 years ago

If you take a look at any of the other scripts, you'll see that I use restructured text. I use Sphinx to create docs from the code. Does that answer your question?

brianthelion commented 9 years ago

@binarybottle Yep! Thanks.

binarybottle commented 8 years ago

@brianthelion -- If you are unable to devote any more time to documentation of the code, shall I close this issue?

satra commented 8 years ago

@binarybottle - was this fixed?

binarybottle commented 8 years ago

Since the only lingering part of the discussion was the question of documentation, I didn't think it helpful to keep this issue open.