Skip to content

fix v2 init issue on Mac #5808

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 22, 2017
Merged

fix v2 init issue on Mac #5808

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e3b250
fix v2 init issue on Mac
tensor-tang Nov 21, 2017
590b0a9
refine the v2 init
tensor-tang Nov 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 59 additions & 24 deletions python/paddle/v2/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,15 @@
cp.begin_parse()


def init(**kwargs):
import py_paddle.swig_paddle as api
args = []
args_dict = {}
# NOTE: append arguments if they are in ENV
for ek, ev in os.environ.iteritems():
if ek.startswith("PADDLE_INIT_"):
args_dict[ek.replace("PADDLE_INIT_", "").lower()] = str(ev)
def auto_set_cpu_env(trainer_count):
'''Auto set CPU environment if have not set before.
export KMP_AFFINITY, OMP_DYNAMIC according to the Hyper Threading status.
export OMP_NUM_THREADS, MKL_NUM_THREADS according to trainer_count.
'''
import platform
if platform.system() != "Linux" and platform.system() != "Darwin":
return

args_dict.update(kwargs)
# NOTE: overwrite arguments from ENV if it is in kwargs
for key in args_dict.keys():
args.append('--%s=%s' % (key, str(args_dict[key])))

# auto set cpu environment
def set_env(key, value):
'''If the key has not been set in the environment, set it with value.'''
assert isinstance(key, str)
Expand All @@ -85,22 +79,63 @@ def set_env(key, value):
if envset is None:
os.environ[key] = value

ht = os.popen("lscpu |grep \"per core\"|awk -F':' '{print $2}'|xargs")
ht = int(ht.read())
if ht == 1: # ht is off
set_env("OMP_DYNAMIC", "false")
set_env("KMP_AFFINITY", "granularity=fine,compact,0,0")
else:
def is_hyperthreading_enabled():
'''If Hyper Threading is enabled.'''
if platform.system() == "Linux":
percore = os.popen(
"lscpu |grep \"per core\"|awk -F':' '{print $2}'|xargs")
return int(percore.read()) != 1
elif platform.system() == "Darwin":
physical = int(os.popen("sysctl hw.physicalcpu").read())
logical = int(os.popen("sysctl hw.logicalcpu").read())
return logical > physical
else:
# do not support on other platform yet
return False

def get_logical_processors():
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function returns the number of logical processors, other than a list of logical processors, so it should be named either get_num_logical_processor or num_logical_processor. Otherwise, we'd lose the most important concept "number".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, thanks, I will use num_logical_processor instead.

'''Get the logical processors number'''
import platform
if platform.system() == "Linux":
processors = os.popen(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly, processors => num_proc or num_processors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

"grep \"processor\" /proc/cpuinfo|sort -u|wc -l")
return int(processors.read())
elif platform.system() == "Darwin":
processors = os.popen("sysctl hw.logicalcpu")
return int(processors.read())
else:
# do not support on other platform yet
return 1
Copy link
Collaborator

@wangkuiyi wangkuiyi Nov 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think this function definition (26 lines, from L82 to L108) can be significantly shortened (into 4 lines) while keeping the readability:

def num_logical_processor():
    cmds = { "Linux" : "grep \"processor\" /proc/cpuinfo|sort -u|wc -l",
             "Darwin" : "sysctl hw.logicalcpu" }
    return int(os.popen(cmds.get(platform.system(), "expr 1")).read())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, Thx.


if is_hyperthreading_enabled():
set_env("OMP_DYNAMIC", "true")
set_env("KMP_AFFINITY", "granularity=fine,compact,1,0")
processors = os.popen("grep \"processor\" /proc/cpuinfo|sort -u|wc -l")
processors = int(processors.read())
trainers = kwargs.get('trainer_count', 1)
threads = processors / trainers
else:
set_env("OMP_DYNAMIC", "false")
set_env("KMP_AFFINITY", "granularity=fine,compact,0,0")
processors = get_logical_processors()
threads = processors / trainer_count
threads = '1' if threads < 1 else str(threads)
set_env("OMP_NUM_THREADS", threads)
set_env("MKL_NUM_THREADS", threads)


def init(**kwargs):
import py_paddle.swig_paddle as api
args = []
args_dict = {}
# NOTE: append arguments if they are in ENV
for ek, ev in os.environ.iteritems():
if ek.startswith("PADDLE_INIT_"):
args_dict[ek.replace("PADDLE_INIT_", "").lower()] = str(ev)

args_dict.update(kwargs)
# NOTE: overwrite arguments from ENV if it is in kwargs
for key in args_dict.keys():
args.append('--%s=%s' % (key, str(args_dict[key])))

auto_set_cpu_env(kwargs.get('trainer_count', 1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

All programs, including auto_set_cpu_env, were written to do something automatically, so it seems that an accurate name could be set_omp_mkl_env_vars.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, thx.


if 'use_gpu' in kwargs:
cp.g_command_config_args['use_gpu'] = kwargs['use_gpu']
if 'use_mkldnn' in kwargs:
Expand Down