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

misc: Add deviceid to configuration and enhance switchconfig #2175

Merged
merged 6 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions devito/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def reinit_compiler(val):
"""
configuration['compiler'].__init__(suffix=configuration['compiler'].suffix,
mpi=configuration['mpi'])
return val


# Setup target platform and compiler
Expand Down Expand Up @@ -95,6 +96,9 @@ def reinit_compiler(val):
# Enable/disable automatic padding for allocated data
configuration.add('autopadding', False, [False, True])

# Select target device
configuration.add('deviceid', -1, preprocessor=int, impacts_jit=False)


def autotune_callback(val): # noqa
if isinstance(val, str):
Expand Down
4 changes: 4 additions & 0 deletions devito/arch/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ def __new_with__(self, **kwargs):
mpi=kwargs.pop('mpi', configuration['mpi']),
**kwargs)

@property
def name(self):
return self.__class__.__name__

@memoized_meth
def get_jit_dir(self):
"""A deterministic temporary directory for jit-compiled objects."""
Expand Down
2 changes: 2 additions & 0 deletions devito/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ def _set_log_level(level):

logger.setLevel(level)

return level


def set_log_level(level, comm=None):
"""
Expand Down
4 changes: 3 additions & 1 deletion devito/operator/operator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,9 @@ def parse_kwargs(**kwargs):

# `allocator`
kwargs['allocator'] = default_allocator(
'%s.%s' % (kwargs['compiler'].__class__.__name__, kwargs['language'])
'%s.%s.%s' % (kwargs['compiler'].name,
kwargs['language'],
kwargs['platform'])
)

return kwargs
45 changes: 21 additions & 24 deletions devito/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,6 @@ def _preprocess(self, key, value):
else:
return value

def _updated(self, key, value):
"""
Execute the callback associated to ``key``, if any.
"""
if key in self._update_functions:
retval = self._update_functions[key](value)
if retval is not None:
super(Parameters, self).__setitem__(key, retval)

@_check_key_deprecation
def __getitem__(self, key, *args):
return super(Parameters, self).__getitem__(key)
Expand All @@ -89,8 +80,10 @@ def __getitem__(self, key, *args):
@_check_key_value
def __setitem__(self, key, value):
value = self._preprocess(key, value)
super(Parameters, self).__setitem__(key, value)
self._updated(key, value)
if key in self._update_functions:
value = self._update_functions[key](value)
if value is not None:
super(Parameters, self).__setitem__(key, value)

@_check_key_deprecation
@_check_key_value
Expand Down Expand Up @@ -232,31 +225,35 @@ def init_configuration(configuration=configuration, env_vars_mapper=env_vars_map
class switchconfig(object):

"""
Decorator to temporarily change `configuration` parameters.
Decorator or context manager to temporarily change `configuration` parameters.
"""

def __init__(self, condition=True, **params):
if condition:
self.params = {k.replace('_', '-'): v for k, v in params.items()}
else:
self.params = {}
self.previous = {}

def __enter__(self, condition=True, **params):
self.previous = {}
for k, v in self.params.items():
self.previous[k] = configuration[k]
configuration[k] = v
Copy link
Contributor

Choose a reason for hiding this comment

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

self.previous = self.params.copy()
configuration.update(**self.params)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this has a slightly different effect, although I could write it more succinctly:

self.previous = {k: configuration[k] for k in self.params.keys()}
configuration.update(**self.params)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, configuration.update(**self.params) doesn't work, since the update method has a different meaning in configuration


def __exit__(self, exc_type, exc_val, exc_tb):
for k, v in self.params.items():
try:
configuration.update(k, self.previous[k])
except ValueError:
# E.g., `platform` and `compiler` will end up here
super(Parameters, configuration).__setitem__(k, self.previous[k])

def __call__(self, func, *args, **kwargs):
@wraps(func)
def wrapper(*args, **kwargs):
previous = {}
for k, v in self.params.items():
previous[k] = configuration[k]
configuration[k] = v
try:
with self:
result = func(*args, **kwargs)
finally:
for k, v in self.params.items():
try:
configuration[k] = previous[k]
except ValueError:
# E.g., `platform` and `compiler` will end up here
configuration[k] = previous[k].name
return result
return wrapper

Expand Down
8 changes: 8 additions & 0 deletions devito/types/parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,14 @@ class DeviceID(DeviceSymbol):
def default_value(self):
return -1

def _arg_values(self, **kwargs):
if self.name in kwargs:
return {self.name: kwargs.pop(self.name)}
elif configuration['deviceid'] != self.default_value:
return {self.name: configuration['deviceid']}
else:
return {self.name: self.default_value}
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just {self.name: kwargs.pop(self.name, self.default_value)} and have default_value return configuration['deviceid']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isn't that what the original code did? I thought the idea was to keep default_value and configuration['deviceid'] separate?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, agree with Carlos here. We can't just return a (mutable) global value from default_value :-)



class DeviceRM(DeviceSymbol):

Expand Down