fix issues raised by Coverity scans (#7431)

This commit combines fixes for 37 potential code issues found in
Coverity scans.
the issues include but are not limited to potential access to
uninitialized variables, dead and redundant code.
We understand that reviewing such a commit can be difficult and will be
happy to help with any questions or changes required.

---------

Signed-off-by: Nir Sonnenschein <[email protected]>
Co-authored-by: Logan Adams <[email protected]>
Co-authored-by: Olatunji Ruwase <[email protected]>

Author: 3 people

accelerator/real_accelerator.py

@@ -67,7 +67,7 @@ def get_accelerator():
                     f"XPU_Accelerator requires intel_extension_for_pytorch, which is not installed on this system.")
         elif accelerator_name == "xpu.external":
             try:
-                import intel_extension_for_deepspeed  # noqa: F401 # type: ignore
+                from intel_extension_for_deepspeed import XPU_Accelerator  # noqa: F401 # type: ignore
             except ImportError as e:
                 raise ValueError(
                     f"XPU_Accelerator external requires intel_extension_for_deepspeed, which is not installed on this system."
@@ -224,6 +224,12 @@ def get_accelerator():
         ds_accelerator = CPU_Accelerator()
     elif accelerator_name == "xpu.external":
         # XPU_Accelerator is already imported in detection stage
+        try:
+            from intel_extension_for_deepspeed import XPU_Accelerator  # noqa: F811
+        except ImportError as e:
+            raise ValueError(
+                f"XPU_Accelerator external requires intel_extension_for_deepspeed, which is not installed on this system."
+            )
         ds_accelerator = XPU_Accelerator()
     elif accelerator_name == "xpu":
         from .xpu_accelerator import XPU_Accelerator
@@ -258,7 +264,7 @@ def get_accelerator():
 def set_accelerator(accel_obj):
     global ds_accelerator
     _validate_accelerator(accel_obj)
-    if accel_logger is not None:
+    if accel_logger is not None and accel_obj is not None:
         accel_logger.info(f"Setting ds_accelerator to {accel_obj._name} (model specified)")
     ds_accelerator = accel_obj
 

deepspeed/autotuning/autotuner.py

@@ -81,7 +81,7 @@ def __init__(self, args, active_resources):
         if not os.path.exists(self.results_dir):
             try:
                 os.makedirs(self.results_dir, exist_ok=True)
-                logger.info(f"Created autotuning results directory: {self.exps_dir}")
+                logger.info(f"Created autotuning results directory: {self.results_dir}")
             except:
                 logger.error(
                     f"Failed to create {self.results_dir}, please check results_dir in the autotuning config file is accessible by all the nodes in the job."

deepspeed/autotuning/constants.py

@@ -144,7 +144,7 @@
     "zero_optimization": {
         "stage": 3
     },
-    "memory_break_down": False
+    "memory_breakdown": False
 }
 
 DEFAULT_TUNING_SPACE_ZERO_0 = {"zero_optimization": {"stage": 0}}

deepspeed/comm/ccl.py

@@ -77,27 +77,12 @@ def run_collective(self, name, **kwargs):
             return CCLHandler(self.ccl_comm_op)
 
     def all_reduce(self, tensor, op=ReduceOp.SUM, group=None, async_op=False):
-        use_caching = False
-        if use_caching:
-            match_id = f"{tensor.size()}-{op}"
-            name = "all_reduce_caching"
-            if name in self.available_coll:
-                group = self.get_all_ranks_from_group(group)
-                return self.ccl_comm_op.all_reduce_caching(tensor, op, match_id, group, async_op)
-            else:
-                return self.run_collective(name=name,
-                                           tensor=tensor,
-                                           op=op,
-                                           match_id=match_id,
-                                           group=group,
-                                           async_op=async_op)
+        name = "all_reduce"
+        if name in self.available_coll:
+            group = self.get_all_ranks_from_group(group)
+            return self.ccl_comm_op.all_reduce(tensor, op, group, async_op)
         else:
-            name = "all_reduce"
-            if name in self.available_coll:
-                group = self.get_all_ranks_from_group(group)
-                return self.ccl_comm_op.all_reduce(tensor, op, group, async_op)
-            else:
-                return self.run_collective(name=name, tensor=tensor, op=op, group=group, async_op=async_op)
+            return self.run_collective(name=name, tensor=tensor, op=op, group=group, async_op=async_op)
 
     def inference_all_reduce(self, tensor, op=ReduceOp.SUM, group=None):
         name = "inference_all_reduce"

deepspeed/compile/passes/offload_activation.py

@@ -101,7 +101,7 @@ def reload_activation_bwd(graph: Graph, graph_id: int, graph_order: List[int], m
         with graph.inserting_after(reload_node):
             wait_node = graph.create_node('call_function',
                                           torch.ops.dc.wait_reload.default, (reload_node, graph_id, val_id), {},
-                                          name=f"wait_copy_{node.name}_{val_id}")
+                                          name=f"wait_copy_{reload_node.name}_{val_id}")
 
         # replace all uses of node with wait_node
         users = {}

deepspeed/compression/helper.py

@@ -137,7 +137,7 @@ def module_replacement(model, module_name, compression_technique=None, mpu=None)
     else:
         new_module = None
 
-    if compression_technique is not None:
+    if compression_technique is not None and new_module is not None:
         for k, v in compression_technique.items():
             if k == SPARSE_PRUNING:
                 if v[SPARSE_PRUNING_ENABLED]:

deepspeed/inference/v2/model_implementations/sharding/qkv.py

@@ -37,12 +37,16 @@ def shard_qkv_param(param: torch.Tensor,
     if n_heads_kv is not None and n_heads_q is None:
         raise ValueError("n_heads_kv should not be passed without n_heads_q")
 
+    if param is None:
+        raise ValueError("param should not be None")
     if n_heads_q is None:
         # Guaranteed to be in MHA
         if param.shape[0] // 3 % head_size != 0:
             raise ValueError("MHA param shape is not correct")
         n_heads_q = param.shape[0] // head_size // 3
         mha_sharding = True
+    elif n_heads_kv is None:
+        mha_sharding = True
     else:
         mha_sharding = n_heads_q == n_heads_kv
 
@@ -73,9 +77,6 @@ def shard_qkv_param(param: torch.Tensor,
         else:
             even_kv_sharding = n_heads_kv >= num_shards
 
-        if param is None:
-            return None
-
         q_param = param[:head_size * n_heads_q]
         kv_param = param[head_size * n_heads_q:]
 

deepspeed/inference/v2/ragged/sequence_descriptor.py

@@ -122,9 +122,9 @@ def __init__(self,
 
         self._seen_tokens = 0
         self._in_flight_tokens = 0
+        assert kv_cache_ids_shadow is not None  # add check before use
 
-        self._num_allocation_groups = tuple(kv_cache_ids_shadow.shape[0]
-                                            for kv_cache_ids_shadow in kv_cache_ids_shadow)
+        self._num_allocation_groups = tuple(kv_cache_id.shape[0] for kv_cache_id in kv_cache_ids_shadow)
         self._blocks_per_allocation_group = tuple(
             torch.zeros(num_groups, dtype=torch.int32, device="cpu") for num_groups in self._num_allocation_groups)
 

deepspeed/module_inject/containers/megatron_gpt.py

@@ -73,6 +73,8 @@ def attention(self, enable_training=False):
                 attention = self.client_module.attention
             else:
                 attention = self.client_module.self_attention
+        else:
+            return None
 
         return attention.query_key_value.weight, \
                attention.query_key_value.bias, \

deepspeed/module_inject/replace_module.py

@@ -93,8 +93,10 @@ def replace_attn(child, policy):
             return child
         if len(policy_attn) == 5:
             qkvw, attn_ow, attn_ob, hidden_size, heads = policy_attn
+            qw, kw, vw = torch.empty(0), torch.empty(0), torch.empty(0)
         else:
             qw, kw, vw, attn_ow, attn_ob, hidden_size, heads = policy_attn
+            qkvw = torch.empty(0)
 
         config = transformer_inference.DeepSpeedInferenceConfig(
             hidden_size=hidden_size,
@@ -113,11 +115,15 @@ def transpose(data):
             return data
 
         if len(policy_attn) == 5:
+            assert qkvw is not None and qkvw.data is not None, "qkvw can't be None"
             attn_module.attn_qkvw.data = transpose(qkvw.data)
         else:
             attn_module.attn_qkvw = None
+            assert qw is not None and qw.data is not None, "qw can't be None"
             attn_module.attn_qw.data = transpose(qw.data)
+            assert kw is not None and kw.data is not None, "kw can't be None"
             attn_module.attn_kw.data = transpose(kw.data)
+            assert vw is not None and vw.data is not None, "vw can't be None"
             attn_module.attn_vw.data = transpose(vw.data)
 
         attn_module.attn_qkvb = None
@@ -316,21 +322,15 @@ def replace_wo_policy(module, all_reduce_linears, prefix="", state_dict=None):
         return _autotp._replace_module(module)
 
     def replace_fn(child, _policy, layer_id=0, prefix="", state_dict=None):
-        training = False  # todo: refactor this part to go in the config
-        if training:
-            # copy relevant state from child -> new module
-            new_module = replace_with_policy(child, _policy, config.triangular_masking)
-
+        # copy relevant state from child -> new module
+        if not is_autotp_training_mode() and config.replace_with_kernel_inject:
+            new_module = replace_with_policy(child,
+                                             _policy,
+                                             config.triangular_masking,
+                                             inference=True,
+                                             layer_id=layer_id)
         else:
-            # copy relevant state from child -> new module
-            if not is_autotp_training_mode() and config.replace_with_kernel_inject:
-                new_module = replace_with_policy(child,
-                                                 _policy,
-                                                 config.triangular_masking,
-                                                 inference=True,
-                                                 layer_id=layer_id)
-            else:
-                new_module = replace_wo_policy(child, _policy, prefix=prefix, state_dict=state_dict)
+            new_module = replace_wo_policy(child, _policy, prefix=prefix, state_dict=state_dict)
 
         return new_module