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

StackOverflowError in sort called on numpy array. #7677

Closed
JaroslavTulach opened this issue Aug 28, 2023 · 2 comments · Fixed by #7780
Closed

StackOverflowError in sort called on numpy array. #7677

JaroslavTulach opened this issue Aug 28, 2023 · 2 comments · Fixed by #7780
Assignees
Labels

Comments

@JaroslavTulach
Copy link
Member

The numpy example can be extended to invoke arr.sort - then it gets into endless loop in EqualscomplexNode.equalsInteropObjectWithMembers where

selfMember: <builtin-method 'PBuiltinFunction numpy.float64.is_integer at 0x1d7c90cd' of 'PythonAbstractNativeObject(com.oracle.truffle.nfi.NFIPointer@6b22e8eb)' objects> 
otherMember: <builtin-method 'PBuiltinFunction numpy.float64.is_integer at 0x1d7c90cd' of 'PythonAbstractNativeObject(com.oracle.truffle.nfi.NFIPointer@ae713af)' objects>

e.g. comparing PythonAbstractNativeObject is repeated many, many times.

@JaroslavTulach
Copy link
Member Author

Reported to GraalVM guys as

bug.

@JaroslavTulach
Copy link
Member Author

Brutal workaround that converts numpy.float* into Double:

diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/syntax/HostValueToEnsoNode.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/syntax/HostValueToEnsoNode.java
index 25d58ecd0b..9e6e74b6df 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/syntax/HostValueToEnsoNode.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/interop/syntax/HostValueToEnsoNode.java
@@ -8,13 +8,17 @@ import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.NeverDefault;
 import com.oracle.truffle.api.dsl.ReportPolymorphism;
 import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.interop.ArityException;
 import com.oracle.truffle.api.interop.InteropLibrary;
 import com.oracle.truffle.api.interop.TruffleObject;
+import com.oracle.truffle.api.interop.UnknownIdentifierException;
 import com.oracle.truffle.api.interop.UnsupportedMessageException;
+import com.oracle.truffle.api.interop.UnsupportedTypeException;
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.nodes.Node;
 import org.enso.interpreter.node.expression.builtin.number.utils.ToEnsoNumberNode;
 import org.enso.interpreter.node.expression.foreign.CoerceNothing;
+import org.enso.interpreter.runtime.data.EnsoObject;
 import org.enso.interpreter.runtime.data.text.Text;
 import org.enso.interpreter.runtime.error.WarningsLibrary;
 
@@ -92,6 +96,37 @@ public abstract class HostValueToEnsoNode extends Node {
     return coerceNothing.execute(o);
   }
 
+  @Specialization(guards = "isNumpyFloat(iop, o)")
+  Object doNumpyFloat(
+      TruffleObject o, @Shared("iop") @CachedLibrary(limit = "3") InteropLibrary iop) {
+    try {
+      var n = iop.invokeMember(o, "__float__");
+      if (iop.fitsInDouble(n)) {
+        return iop.asDouble(n);
+      }
+    } catch (UnsupportedMessageException
+        | ArityException
+        | UnknownIdentifierException
+        | UnsupportedTypeException ex) {
+    }
+    return o;
+  }
+
+  static boolean isNumpyFloat(InteropLibrary iop, Object o) {
+    if (o instanceof EnsoObject) {
+      return false;
+    }
+    try {
+      if (iop.hasMetaObject(o)) {
+        var meta = iop.getMetaObject(o);
+        var fqn = iop.asString(iop.getMetaQualifiedName(meta));
+        return fqn.startsWith("numpy.float");
+      }
+    } catch (UnsupportedMessageException ex) {
+    }
+    return false;
+  }
+
   @Fallback
   Object doOther(Object o) {
     return o;

@JaroslavTulach JaroslavTulach moved this from 🔧 Implementation to ⚙️ Design in Issues Board Sep 7, 2023
@JaroslavTulach JaroslavTulach moved this from ⚙️ Design to 👁️ Code review in Issues Board Sep 11, 2023
@JaroslavTulach JaroslavTulach moved this from 👁️ Code review to 🔧 Implementation in Issues Board Sep 15, 2023
@jdunkerley jdunkerley moved this from 🔧 Implementation to 👁️ Code review in Issues Board Sep 19, 2023
@mergify mergify bot closed this as completed in #7780 Sep 19, 2023
mergify bot pushed a commit that referenced this issue Sep 19, 2023
Closes #7677 by eliminating the _stackoverflow execption_. In general it seems _too adventurous_ to walk members of random foreign objects. There can be anything including cycles. Rather than trying to be too smart in these cases, let's just rely on `InteropLibrary.isIdentical` message.

# Important Notes
Calling `sort` on the `numpy` array no longer yields an error, but the array isn't sorted - that needs a fix on the Python side: oracle/graalpython#354 - once it is in, the elements will be treated as numbers and the sorting happens automatically (without any changes in Enso code).
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant