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

Jwang 445 install pure python modules only #447

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jialeiwang
Copy link
Contributor

********* PEOPLE *************
Primary reviewer:

Reviewers:

********* DESCRIPTION **************
Branch Name: jwang_445_install_pure_python_modules_only
Ticket(s)/Issue(s): Closes #445

********* TESTING DONE *************
make test for both MOE_NO_BUILD_CPP=True and MOE_NO_BUILD_CPP=False
Caveat: when MOE_NO_BUILD_CPP=True, unit tests of optimal learning functions for web interface are disabled because currently this part largely depends on C++ components and I shut it down when C++ components are not installed.

@suntzu86
Copy link
Contributor

suntzu86 commented Nov 4, 2015

Sorry I've been gone for so long! See: #448

First, python_version has incomplete feature coverage. It was originally intended for education and prototyping. For example, hyperparameter optimization is extremely slow in Python, plus newton's method isn't even there. Other python GP packages (e.g., spearmint) take entirely different approaches to hyperparameter opt presumably for the performance issue. So that's why it wasn't built with this use case in mind.

That said, this seems like a fairly unpythonic/hacky way to remove the C++ requirement. python_version and cpp_wrappers were meant to be set up like DLLs. So you can invoke
cpp_wrappers.Foo(...)
or
python_version.Foo(...)
with the same arguments and get the same results.

A common python pattern is:

try:
  import A as name
except ImportError:
  import B as name

So if cpp_wrappers import fails (possibly have to catch file not found too?), we import python_version instead. (Name could be "gpp_lib" or something generic/common to both.)

Something slightly different would be needed for the unit tests (skip all the cpp tests if cpp doesn't exist), which would probably be done in __init__.py but I'm not sure.

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

Successfully merging this pull request may close these issues.

2 participants