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

Fixes PFN CI tests #605

Merged
merged 17 commits into from
Nov 25, 2020
Merged

Fixes PFN CI tests #605

merged 17 commits into from
Nov 25, 2020

Conversation

prabhatnagarajan
Copy link
Contributor

@prabhatnagarajan prabhatnagarajan commented Nov 15, 2020

This fixes the build and changes the supported Python version to 3.6.

@prabhatnagarajan prabhatnagarajan changed the title Fixes tests Fixes PFN CI tests Nov 15, 2020
@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 3987d4d:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 3987d4d:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 7ea0292:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 4a3a2c6:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 347fe16:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit d54c60b:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 7eb7d63:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 8839092:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit d8ffda2:

@@ -206,7 +206,6 @@ def make_env(idx, test):
maxSteps=max_episode_steps,
isTest=test,
)
assert env.observation_space is None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this line after this recent update to the environment: bulletphysics/bullet3@3a6a279#diff-37edfb1b0750fd6a650da86f2d5c1ec3827623de1abb22ba2fc7cf897eeb5c44

@muupan muupan self-requested a review November 18, 2020 01:43
@@ -1,9 +1,6 @@
import argparse
import os

# Prevent numpy from using multiple threads
os.environ['OMP_NUM_THREADS'] = '1' # NOQA

Copy link
Member

Choose a reason for hiding this comment

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

You should not move this line. This must be done before import numpy.

@@ -1,9 +1,6 @@
import argparse
import os

# Prevent numpy from using multiple threads
os.environ['OMP_NUM_THREADS'] = '1' # NOQA

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -1,9 +1,6 @@
import argparse
import os

# Prevent numpy from using multiple threads
os.environ['OMP_NUM_THREADS'] = '1' # NOQA

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -2,9 +2,6 @@
import os
import random

# This prevents numpy from using multiple threads
os.environ['OMP_NUM_THREADS'] = '1' # NOQA

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -12,9 +12,6 @@
import argparse
import os

# This prevents numpy from using multiple threads
os.environ['OMP_NUM_THREADS'] = '1' # NOQA

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -1,9 +1,6 @@
import argparse
import os

# This prevents numpy from using multiple threads
os.environ['OMP_NUM_THREADS'] = '1' # NOQA

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

examples/gym/train_pcl_gym.py Outdated Show resolved Hide resolved
@muupan
Copy link
Member

muupan commented Nov 18, 2020

Can you explain why it needs python 3.6?

@prabhatnagarajan
Copy link
Contributor Author

Some of the previous PR CIs were failing:
https://ci.preferred.jp/chainerrl.py3.cpu/62915/

2020-09-16 16:48:46.403755 STDOUT 2092] | Downloading https://files.pythonhosted.org/packages/53/10/776750d57ade26522478a92a2e14035868624a6a62f4157b0cc5abd4a980/scipy-1.5.2.tar.gz (25.4MB) | 5.1s
-- | -- | --
2020-09-16 16:48:51.473617 STDOUT 2092] | Complete output from command python setup.py egg_info: |  
2020-09-16 16:48:51.473630 STDOUT 2092] | Traceback (most recent call last): |  
2020-09-16 16:48:51.473644 STDOUT 2092] | File "<string>", line 1, in <module> |  
2020-09-16 16:48:51.473747 STDOUT 2092] | File "/tmp/pip-build-sxosch92/scipy/setup.py", line 31, in <module> |  
2020-09-16 16:48:51.473749 STDOUT 2092] | raise RuntimeError("Python version >= 3.6 required.") |  
2020-09-16 16:48:51.473756 STDOUT 2092] | RuntimeError: Python version >= 3.6 required.

I thought it would be ok to update the required version to 3.6 (at least for testing).

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 7b2fabb:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 484f12e:

@prabhatnagarajan
Copy link
Contributor Author

/test

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit c41db82:

Copy link
Member

@muupan muupan left a comment

Choose a reason for hiding this comment

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

While E402 can be addressed in a different way (https://gitlab.com/pycqa/flake8/-/issues/638), I think it is good to merge as it is since fixing the CI asap is important.

@muupan muupan merged commit 736475e into chainer:master Nov 25, 2020
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.

3 participants