gemini-code-assist[bot] commented on code in PR #18773:
URL: https://github.com/apache/tvm/pull/18773#discussion_r2803227050


##########
python/tvm/relax/frontend/onnx/onnx_frontend.py:
##########
@@ -1181,15 +1181,27 @@ def _impl_v1(cls, bb, inputs, attr, params):
 
 
 class Gelu(OnnxOpConverter):
-    """Operator converter for Gelu from Microsoft onnxruntime contrib opset.
+    """Operator converter for Gelu.
+
+    Supports both Microsoft onnxruntime contrib opset and ONNX Opset 20+.
 
     gelu(x) = 0.5x(1 + erf(x/sqrt(2)))
+
+    When approximate="tanh" (ONNX Opset 20):
+    gelu(x) = 0.5x(1 + tanh(sqrt(2/pi)(x + 0.044715x^3)))
     """
 
     @classmethod
     def _impl_v1(cls, bb, inputs, attr, params):
         return relax.op.nn.gelu(inputs[0])
 
+    @classmethod
+    def _impl_v20(cls, bb, inputs, attr, params):
+        approximate = attr.get("approximate", "none")
+        if approximate == "tanh":
+            return relax.op.nn.gelu_tanh(inputs[0])
+        return relax.op.nn.gelu(inputs[0])

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   While the current implementation correctly handles the 'tanh' approximation, 
it silently falls back to the default 'erf' implementation for any other value 
of `approximate` besides 'tanh'. According to the ONNX specification for Gelu, 
the only valid values for `approximate` are 'tanh' and 'none'.
   
   To make the implementation more robust and explicit, it would be better to 
raise an error for any unsupported values. This aligns with how other operators 
with string attributes are handled in this file (e.g., `BitShift`).
   
   ```suggestion
           approximate = attr.get("approximate", "none")
           if approximate == "tanh":
               return relax.op.nn.gelu_tanh(inputs[0])
           elif approximate == "none":
               return relax.op.nn.gelu(inputs[0])
           else:
               raise ValueError(f"Unsupported approximate mode for Gelu: 
{approximate}")
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to