Merge pull request #2168 from ekouts/bugfix/autodetect_processor_info

[bugfix] Fix incomplete processor info object when processor topology is auto-detected

Author: Vasileios Karakasis

reframe/core/systems.py

@@ -13,30 +13,50 @@
 from reframe.core.modules import ModulesSystem
 
 
-class ProcessorType(jsonext.JSONSerializable):
+class _ReadOnlyInfo:
+    __slots__ = ('_info',)
+    _known_attrs = ()
+
+    def __init__(self, info):
+        self._info = info
+
+    def __deepcopy__(self, memo):
+        # This is a read-only object; simply return ourself
+        return self
+
+    def __getattr__(self, name):
+        if name in self._known_attrs:
+            return self._info.get(name, None)
+        else:
+            raise AttributeError(
+                f'{type(self).__qualname__!r} object has no attribute {name!r}'
+            )
+
+    def __setattr__(self, name, value):
+        if name in self._known_attrs:
+            raise AttributeError(f'attribute {name!r} is not writeable')
+        else:
+            super().__setattr__(name, value)
+
+
+class ProcessorInfo(_ReadOnlyInfo, jsonext.JSONSerializable):
     '''A representation of a processor inside ReFrame.
 
+    You can access all the keys of the `processor configuration object
+    <config_reference.html#processor-info>`__.
+
     .. versionadded:: 3.5.0
 
     .. warning::
-       Users may not create :class:`ProcessorType` objects directly.
+       Users may not create :class:`ProcessorInfo` objects directly.
 
     '''
 
-    def __init__(self, processor_info):
-        self._arch = None
-        self._num_cpus = None
-        self._num_cpus_per_core = None
-        self._num_cpus_per_socket = None
-        self._num_sockets = None
-        self._topology = None
-        self._info = processor_info
-
-        if not processor_info:
-            return
-
-        for key, val in processor_info.items():
-            setattr(self, f'_{key}', val)
+    __slots__ = ()
+    _known_attrs = (
+        'arch', 'num_cpus', 'num_cpus_per_core',
+        'num_cpus_per_socket', 'num_sockets', 'topology'
+    )
 
     @property
     def info(self):
@@ -46,62 +66,14 @@ def info(self):
         '''
         return self._info
 
-    @property
-    def arch(self):
-        '''The microarchitecture of the processor.
-
-        :type: :class:`str` or :class:`None`
-        '''
-        return self._arch
-
-    @property
-    def num_cpus(self):
-        '''Number of logical CPUs.
-
-        :type: integral or :class:`None`
-        '''
-        return self._num_cpus
-
-    @property
-    def num_cpus_per_core(self):
-        '''Number of logical CPUs per core.
-
-        :type: integral or :class:`None`
-        '''
-        return self._num_cpus_per_core
-
-    @property
-    def num_cpus_per_socket(self):
-        '''Number of logical CPUs per socket.
-
-        :type: integral or :class:`None`
-        '''
-        return self._num_cpus_per_socket
-
-    @property
-    def num_sockets(self):
-        '''Number of sockets.
-
-        :type: integral or :class:`None`
-        '''
-        return self._num_sockets
-
-    @property
-    def topology(self):
-        '''Processor topology.
-
-        :type: :class:`Dict[str, obj]` or :class:`None`
-        '''
-        return self._topology
-
     @property
     def num_cores(self):
         '''Total number of cores.
 
         :type: integral or :class:`None`
         '''
-        if self._num_cpus and self._num_cpus_per_core:
-            return self._num_cpus // self._num_cpus_per_core
+        if self.num_cpus and self.num_cpus_per_core:
+            return self.num_cpus // self.num_cpus_per_core
         else:
             return None
 
@@ -111,8 +83,8 @@ def num_cores_per_socket(self):
 
         :type: integral or :class:`None`
         '''
-        if self.num_cores and self._num_sockets:
-            return self.num_cores // self._num_sockets
+        if self.num_cores and self.num_sockets:
+            return self.num_cores // self.num_sockets
         else:
             return None
 
@@ -122,8 +94,8 @@ def num_numa_nodes(self):
 
         :type: integral or :class:`None`
         '''
-        if self._topology and 'numa_nodes' in self._topology:
-            return len(self._topology['numa_nodes'])
+        if self.topology and 'numa_nodes' in self.topology:
+            return len(self.topology['numa_nodes'])
         else:
             return None
 
@@ -140,37 +112,21 @@ def num_cores_per_numa_node(self):
             return None
 
 
-class DeviceType(jsonext.JSONSerializable):
+class DeviceInfo(_ReadOnlyInfo, jsonext.JSONSerializable):
     '''A representation of a device inside ReFrame.
 
+    You can access all the keys of the `device configuration object
+    <config_reference.html#device-info>`__.
+
     .. versionadded:: 3.5.0
 
     .. warning::
-       Users may not create :class:`DeviceType` objects directly.
+       Users may not create :class:`DeviceInfo` objects directly.
 
     '''
 
-    def __init__(self, device_info):
-        self._type = None
-        self._arch = None
-        self._num_devices = 1
-        self._info = device_info
-
-        if not device_info:
-            return
-
-        for key, val in device_info.items():
-            setattr(self, f'_{key}', val)
-
-    @property
-    def num_devices(self):
-        '''Number of devices of this type.
-
-        It will return 1 if it wasn't set in the configuration.
-
-        :type: integral
-        '''
-        return self._num_devices
+    __slots__ = ()
+    _known_attrs = ('type', 'arch')
 
     @property
     def info(self):
@@ -181,20 +137,22 @@ def info(self):
         return self._info
 
     @property
-    def arch(self):
-        '''The architecture of the device.
+    def num_devices(self):
+        '''Number of devices of this type.
 
-        :type: :class:`str` or :class:`None`
+        It will return 1 if it wasn't set in the configuration.
+
+        :type: integral
         '''
-        return self._arch
+        return self._info.get('num_devices', 1)
 
     @property
     def device_type(self):
         '''The type of the device.
 
         :type: :class:`str` or :class:`None`
         '''
-        return self._type
+        return self.type
 
 
 class SystemPartition(jsonext.JSONSerializable):
@@ -222,8 +180,8 @@ def __init__(self, parent, name, sched_type, launcher_type,
         self._max_jobs = max_jobs
         self._prepare_cmds = prepare_cmds
         self._resources = {r['name']: r['options'] for r in resources}
-        self._processor = ProcessorType(processor)
-        self._devices = [DeviceType(d) for d in devices]
+        self._processor = ProcessorInfo(processor)
+        self._devices = [DeviceInfo(d) for d in devices]
         self._extras = extras
 
     @property
@@ -390,7 +348,7 @@ def processor(self):
 
         .. versionadded:: 3.5.0
 
-        :type: :class:`reframe.core.systems.ProcessorType`
+        :type: :class:`reframe.core.systems.ProcessorInfo`
         '''
         return self._processor
 
@@ -400,7 +358,7 @@ def devices(self):
 
         .. versionadded:: 3.5.0
 
-        :type: :class:`List[reframe.core.systems.DeviceType]`
+        :type: :class:`List[reframe.core.systems.DeviceInfo]`
         '''
         return self._devices
 

reframe/frontend/autodetect.py

@@ -15,6 +15,7 @@
 from reframe.core.exceptions import ConfigError
 from reframe.core.logging import getlogger
 from reframe.core.schedulers import Job
+from reframe.core.systems import DeviceInfo, ProcessorInfo
 from reframe.utility.cpuinfo import cpuinfo
 
 
@@ -183,8 +184,8 @@ def detect_topology():
                 f'> found topology file {topo_file!r}; loading...'
             )
             try:
-                part.processor._info = _load_info(
-                    topo_file, _subschema('#/defs/processor_info')
+                part._processor = ProcessorInfo(
+                    _load_info(topo_file, _subschema('#/defs/processor_info'))
                 )
                 found_procinfo = True
             except json.decoder.JSONDecodeError as e:
@@ -197,9 +198,10 @@ def detect_topology():
                 f'> found devices file {dev_file!r}; loading...'
             )
             try:
-                part._devices = _load_info(
+                devices_info = _load_info(
                     dev_file, _subschema('#/defs/devices')
                 )
+                part._devices = [DeviceInfo(d) for d in devices_info]
                 found_devinfo = True
             except json.decoder.JSONDecodeError as e:
                 getlogger().debug(
@@ -220,13 +222,13 @@ def detect_topology():
 
                 # Unconditionally detect the system for fully local partitions
                 with runtime.temp_environment(modules=modules, variables=vars):
-                    part.processor._info = cpuinfo()
+                    part._processor = ProcessorInfo(cpuinfo())
 
                 _save_info(topo_file, part.processor.info)
             elif detect_remote_systems:
                 with runtime.temp_environment(modules=temp_modules,
                                               variables=temp_vars):
-                    part.processor._info = _remote_detect(part)
+                    part._processor = ProcessorInfo(_remote_detect(part))
 
                 if part.processor.info:
                     _save_info(topo_file, part.processor.info)

reframe/utility/jsonext.py

@@ -12,12 +12,20 @@
 
 
 class JSONSerializable:
+    __slots__ = ()
+
     def __rfm_json_encode__(self):
         ret = {
             '__rfm_class__': type(self).__qualname__,
             '__rfm_file__': inspect.getfile(type(self))
         }
-        ret.update(self.__dict__)
+        if hasattr(self, '__dict__'):
+            ret.update(self.__dict__)
+
+        # Set the slots attribute
+        for attr in self.__slots__:
+            ret[attr] = getattr(self, attr)
+
         encoded_ret = encode_dict(ret, recursive=True)
         return encoded_ret if encoded_ret else ret
 
@@ -90,7 +98,12 @@ def _object_hook(json):
     mod = util.import_module_from_file(filename)
     cls = getattr(mod, typename)
     obj = cls.__new__(cls)
-    obj.__dict__.update(json)
+    if hasattr(obj, '__dict__'):
+        obj.__dict__.update(json)
+    else:
+        for attr, value in json.items():
+            setattr(obj, attr, value)
+
     if hasattr(obj, '__rfm_json_decode__'):
         obj.__rfm_json_decode__(json)
 

unittests/test_autodetect.py

@@ -54,7 +54,29 @@ def test_autotect(exec_ctx):
     detect_topology()
     part = runtime().system.partitions[0]
     assert part.processor.info == cpuinfo()
-    assert part.devices == [{'type': 'gpu', 'arch': 'a100', 'num_devices': 8}]
+    if part.processor.info:
+        assert part.processor.num_cpus == part.processor.info['num_cpus']
+
+    assert len(part.devices) == 1
+    assert part.devices[0].info == {
+        'type': 'gpu',
+        'arch': 'a100',
+        'num_devices': 8
+    }
+    assert part.devices[0].device_type == 'gpu'
+
+    # Test immutability of ProcessorInfo and DeviceInfo
+    with pytest.raises(AttributeError):
+        part.processor.num_cpus = 3
+
+    with pytest.raises(AttributeError):
+        part.processor.foo = 10
+
+    with pytest.raises(AttributeError):
+        part.devices[0].arch = 'foo'
+
+    with pytest.raises(AttributeError):
+        part.devices[0].foo = 10
 
 
 def test_autotect_with_invalid_files(invalid_topo_exec_ctx):