Yelp / MOE

A global, black box optimization engine for real world metric optimization.
Other
1.31k stars 139 forks source link

Eliu gh 353 make autodoc pipeline recognize new gpu docs #363

Closed suntzu86 closed 10 years ago

suntzu86 commented 10 years ago

***** PEOPLE ***** Primary reviewer: @sc932

Reviewers: @garywang304

***** DESCRIPTION ** Branch Name: eliu_gh_353_make_autodoc_pipeline_recognize_new_gpu_docs Ticket(s)/Issue(s): Closes #353

1. define version string
2. make autodoc tools understand the gpu directory
3. cleanup some doc warnings & rendering issues
4. updating contributing.rst to include version bump steps

@sc932 major files to see are any .md and .rst files, .py files (conf, setup, etc), doxygen_config, and cpp_rst_maker.py.

hpp/cpp changes are all doc changes.

@garywang304 please check over changes to the *gpu* files and *cuda* files as well as cmake.

edit: added some file specs to check

***** TESTING DONE ***** make test make -B docs (visual checks)

suntzu86 commented 10 years ago

Travis is probably going to fail. This appears to be b/c Travis doesn't know what pyramid is when it builds the docs. This issue existed previously but now it kills the build b/c I cannot import moe (init.py does pyramid stuff in addition to holding the version string).

@sc932 thoughts?

Also @sc932 : i see we have version info in setup.py as well. This shouldn't be multiply defined. It appears to be much more standard to keep the version in moe/__init__.py but this might not be import-able from setup.py since it does pyramid stuff? edit: on second thought, I think this won't be an issue b/c ppl should pip install reqs before running setup.

suntzu86 commented 10 years ago

@sc932 @Feriority @ypwais The particular error is:

  File "/usr/local/lib/python2.7/dist-packages/pyramid/registry.py", line 5, in <module>
    from zope.interface.registry import Components
ImportError: No module named registry

This might be another instance of an older linux screwing us. zope.interface.registry came into existence in version 3.8.0: https://pypi.python.org/pypi/zope.interface/3.8.0 (latest is 4.1.1) In fact pyramid has this listed as a version requirement in their setup.py. Looking through the travis log, it does download 4.1.1 then sees 3.6.1 installed and uninstalls the old one (in theory...)

Travis seems to indicate that installing python via apt is bad: http://docs.travis-ci.com/user/languages/python/

So possible ideas:

  1. put zope in our requirements.txt explicitly. Initially I thought this felt dirty since it's a 3rd party req (and is listed in pyramid!) but the pip docs seem to encourage it: https://pip.readthedocs.org/en/1.1/requirements.html#requirements-files (end of section)
  2. stop installing python with apt-get
  3. pip install/upgrade/whatever zope in .travis.yaml explicitly (this feels the ickiest?)

OR Travis isn't running make docs in the virtualenv...? (b/c how else does this failure not occur earlier?!)

suntzu86 commented 10 years ago

doxygen also pukes on travis (in a non-build-killing way) due to an out of date (12.04) version. the latest version is available through ppa for 12.04 though, i think: http://www.ubuntuupdates.org/package/libreoffice/precise/main/base/doxygen while i'm fixing this might as well include that

sc932 commented 10 years ago

This part looks fine (pending single comment).

Let me know when you finish it up.

sc932 commented 10 years ago

A few random comments, good overall :)

jialeiwang commented 10 years ago

Changes to GPU related files and CMake file look all good to me

suntzu86 commented 10 years ago

review response up. @sc932 @garywang304 please have a look!

sc932 commented 10 years ago

Ship it pending green. Can you make a ticket for moving from code -> latex where it makes sense? Some of these matrix ops look like crap as latex as verbatim code :(

Seems like a good plane ticket.

jialeiwang commented 10 years ago

cmake files for both cpp and gpu are good. I also compiled the code with GPU switch turned on and it had no problem.

suntzu86 commented 10 years ago

@sc932 https://github.com/Yelp/MOE/issues/377 @garywang304 thanks!