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

Compilation error workaround #3767

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Compilation error workaround #3767

merged 2 commits into from
Jan 14, 2020

Conversation

fonnesbeck
Copy link
Member

This applies platform-specific flags (for Mac and Windows) to circumvent compilation errors for certain models.

Closes #3695

@fonnesbeck fonnesbeck requested a review from ColCarroll January 10, 2020 12:54
@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #3767 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3767   +/-   ##
=======================================
  Coverage   90.63%   90.63%           
=======================================
  Files         133      133           
  Lines       20328    20328           
=======================================
  Hits        18424    18424           
  Misses       1904     1904

@@ -28,9 +36,18 @@
from .tests import test

from .data import *
import theano
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to go, but I would wrap the whole thing in a mangled function, so that the top-level namespace does not get too busy.

def __set_theano_flags():
    import theano
    import platform
    ...

__set_theano_flags()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

@ColCarroll ColCarroll merged commit 4cb5942 into pymc-devs:master Jan 14, 2020
@ColCarroll
Copy link
Member

It doesn't feel great setting flags by default, but it seems like this is what's needed to get the project to work. A quick search shows that there were at least 3 issues in the last 4 months that this would have fixed.

@fonnesbeck
Copy link
Member Author

Given that we can't do much with Theano, its probably the best course of action.

sthagen added a commit to sthagen/pymc-devs-pymc that referenced this pull request Jan 15, 2020
mihaic added a commit to mihaic/brainiak that referenced this pull request May 21, 2020
mihaic added a commit to brainiak/brainiak that referenced this pull request May 21, 2020
The renames do not affect the API, except for `concatenate_not_none`,
where the rename actually makes the code agree to the existing
documentation.

The existing variable names caused errors after an update of
Pycodestyle in Flake8:
#464
PyCQA/pycodestyle#853
https://gitlab.com/pycqa/flake8/-/merge_requests/418

Also add a workaround for a Theano compilation error, inspired by PyMC3:
pymc-devs/pymc#3767
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.

Theano compilation error when using marginal Gaussian processes
2 participants