Special array representation #148

LakeBlair · 2022-12-04T22:09:10Z

Overview

Added special array representation for np.zeros and np.identity.

Details

References

Blocked by

odashi

Thanks! It looks this change needs some refactoring to support the feature and error handling completely. Which would you like either:

fix them by yourself (in this case I will put comprehensive comments)
delegate remaining tasks to me (your contribution is still recorded correctly)

odashi · 2022-12-05T01:19:23Z

examples/examples.ipynb

@@ -2,41 +2,20 @@
 "cells": [


(action not required) I guess this notebook file is no longer necessary as we could provide a comprehensive examples in Google Colab. I will remove this file later.

odashi · 2022-12-05T01:28:12Z

src/latexify/codegen/function_codegen.py

@@ -356,6 +356,17 @@ def visit_Call(self, node: ast.Call) -> str:
            (default_func_str + r"\mathopen{}\left(", r"\mathclose{}\right)"),
        )

+        if func_name == "zeros":


For zeros, We don't need to constrain only a particular number of dimensions.

I think these processes don't work if the subtree has unexpected syntax. It usually happens when users gave other functions with the same name. As the AST varies, we basically need complete check of the underlying structure of the given subtree.

odashi · 2022-12-05T05:38:13Z

@LakeBlair Let's continue developing this pull request as you preferred in another thread.

Before providing additional comments, please resolve the following issues:

Merge main into this branch and resolve all conflicts.
run ./check.sh and resolve all errors.

LakeBlair · 2022-12-05T16:01:04Z

@LakeBlair Let's continue developing this pull request as you preferred in another thread.

Before providing additional comments, please resolve the following issues:

Merge main into this branch and resolve all conflicts.

run ./check.sh and resolve all errors.

Just did both. Check if they look good to you.

odashi · 2022-12-06T04:09:51Z

src/latexify/codegen/function_codegen.py

@@ -425,7 +425,26 @@ def visit_Call(self, node: ast.Call) -> str:

        if special_latex is not None:
            return special_latex
-
+
+        # Special treatment for np.zeros


In the current implementation, special treatments for several functions are implemented in separate functions (see L419-L427). They return str | None, and visit_Call attempts early returning if str is returned by the function. The functionality of this pull request should be implemented similarly:

The function returns str, the correct LaTeX, if the given ast.Call has a supported syntax.

Otherwise the function returns None, then visit_Call falls back to the default behavior.

odashi · 2022-12-06T04:45:08Z

src/latexify/codegen/function_codegen.py

+            str = ""
+            open_bracket = "{"
+            close_bracket = "}"
+            for i, elt in enumerate(node.args[0].elts):
+                str += "{" + self.visit(elt) + "}"
+                if i != len(node.args[0].elts) - 1:
+                    str += " " + r"\times" + " "
+
+            matrix_str = "0^" + open_bracket + str + close_bracket
+            return matrix_str


General suggestions:

Requires complete syntax checking.

Don't use str as a variable as it is reserved by the builtin type name. Overwriting builtins will confuse the behavior of the code (even if the current code doesn't involve any errors, it will happen in the future). Use latex instead if you need to store some generated strings.

Suppress string concatenation as it is significantly expensive. Making a sequence (generator), then join all of them is generally better.

I think \mathbf{0} ( $\mathbf{0}$ ) should be used to distinguish that this is not a scalar $0$.

Suggested change

str = ""

open_bracket = "{"

close_bracket = "}"

for i, elt in enumerate(node.args[0].elts):

str += "{" + self.visit(elt) + "}"

if i != len(node.args[0].elts) - 1:

str += " " + r"\times" + " "

matrix_str = "0^" + open_bracket + str + close_bracket

return matrix_str

if len(node.args) != 1:

# fall back to the default.

arg0 = node.args[0]

if not isinstance(arg0, ast.Tuple):

# fall back to the default.

# Tecunically we don't support `zeros(n)` where `n` is a scalar.

dims_latex = r" \times ".join(self.visit(x) for x in arg0.elts)

return fr"\mathbf{{0}}^{{{dims_latex}}}"

@odashi I made some changes to my code.

np.zeros and np.identity are now in a new function, treated as special numpy methods.

I added some if statements checking for valid ast inputs.

I added some parametric tests.

Some more considerations:

Some string concatenation is simplified, some could be simplified further.

In the case of np.zeros(0), I just represent it as $\mathbf{0}^{{1} \times {0}}$. There might be better ways of expressing it as a special case. Do you have any suggestions?

Let me know what you think.

odashi · 2022-12-06T04:48:01Z

src/latexify/codegen/function_codegen.py

+            str = "{" + self.visit(node.args[0]) + "}"
+            matrix_str = f"I_{str}"
+            return matrix_str


Same suggestions here.

Suggested change

str = "{" + self.visit(node.args[0]) + "}"

matrix_str = f"I_{str}"

return matrix_str

if len(node.args) != 1:

# fall back to the default.

return fr"\mathbf{I}_{{{self.visit(node.args[0])}}}"

odashi · 2022-12-06T04:51:02Z

src/latexify/codegen/function_codegen_test.py

+    tree = ast.parse(
+        textwrap.dedent(
+            """
+        def f(a, b):
+            return np.zeros((a,b))
+            """
+        )
+    ).body[0]
+    latex = "f(a, b) = 0^{{a} \\times {b}}"
+    assert isinstance(tree, ast.FunctionDef)


We don't need complete parsing. It is enough to obtain only ast.Call.

Suggested change

tree = ast.parse(

textwrap.dedent(

"""

def f(a, b):

return np.zeros((a,b))

"""

)

).body[0]

latex = "f(a, b) = 0^{{a} \\times {b}}"

assert isinstance(tree, ast.FunctionDef)

tree = ast_utils.parse_expr("zeros((a, b))")

assert isinstance(tree, ast.Call)

odashi · 2022-12-06T04:52:32Z

src/latexify/codegen/function_codegen_test.py

@@ -862,6 +862,34 @@ def test_use_set_symbols_compare(code: str, latex: str) -> None:
    assert function_codegen.FunctionCodegen(use_set_symbols=True).visit(tree) == latex


+def test_generate_numpy_zeros():


Please write comprehensive tests to cover every edge case, as in other parametric tests.

odashi · 2022-12-06T04:52:46Z

src/latexify/codegen/function_codegen_test.py

+    assert FunctionCodegen().visit(tree) == latex
+
+
+def test_generate_numpy_identity():


odashi

Sorry I still think this change requires several refactoring, I will do it by my side.

LakeBlair requested a review from odashi as a code owner December 4, 2022 22:09

odashi reviewed Dec 5, 2022

View reviewed changes

odashi reviewed Dec 6, 2022

View reviewed changes

odashi added the feature label Dec 7, 2022

odashi added this to the v0.3 milestone Dec 7, 2022

LakeBlair added 2 commits December 7, 2022 22:20

modified files

19bfd68

minor style fix

2a48c6b

LakeBlair force-pushed the special_array branch 2 times, most recently from acf41ab to 19bfd68 Compare December 8, 2022 03:36

odashi reviewed Dec 8, 2022

View reviewed changes

refactoring

0a81566

odashi approved these changes Dec 9, 2022

View reviewed changes

odashi merged commit e51a619 into google:main Dec 9, 2022

LakeBlair mentioned this pull request Dec 10, 2022

Factor out expression codegen from function codegen #155

Merged

ZibingZhang pushed a commit to ZibingZhang/latexify_py that referenced this pull request Dec 10, 2022

readds changes from google#148

d0f2d98

ZibingZhang mentioned this pull request Dec 10, 2022

Fixes issues from factoring out expression codegen #159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Special array representation #148

Special array representation #148

LakeBlair commented Dec 4, 2022

odashi left a comment

odashi Dec 5, 2022

odashi Dec 5, 2022

odashi commented Dec 5, 2022

LakeBlair commented Dec 5, 2022

odashi Dec 6, 2022 •

edited

Loading

odashi Dec 6, 2022

LakeBlair Dec 8, 2022

odashi Dec 6, 2022

odashi Dec 6, 2022

odashi Dec 6, 2022 •

edited

Loading

odashi Dec 6, 2022

odashi left a comment

		@@ -862,6 +862,34 @@ def test_use_set_symbols_compare(code: str, latex: str) -> None:
		assert function_codegen.FunctionCodegen(use_set_symbols=True).visit(tree) == latex


		def test_generate_numpy_zeros():

		assert FunctionCodegen().visit(tree) == latex


		def test_generate_numpy_identity():

Special array representation #148

Special array representation #148

Conversation

LakeBlair commented Dec 4, 2022

Overview

Details

References

Blocked by

odashi left a comment

Choose a reason for hiding this comment

odashi Dec 5, 2022

Choose a reason for hiding this comment

odashi Dec 5, 2022

Choose a reason for hiding this comment

odashi commented Dec 5, 2022

LakeBlair commented Dec 5, 2022

odashi Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

odashi Dec 6, 2022

Choose a reason for hiding this comment

LakeBlair Dec 8, 2022

Choose a reason for hiding this comment

odashi Dec 6, 2022

Choose a reason for hiding this comment

odashi Dec 6, 2022

Choose a reason for hiding this comment

odashi Dec 6, 2022 • edited Loading

Choose a reason for hiding this comment

odashi Dec 6, 2022

Choose a reason for hiding this comment

odashi left a comment

Choose a reason for hiding this comment

odashi Dec 6, 2022 •

edited

Loading

odashi Dec 6, 2022 •

edited

Loading