Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

adaptation to Python 3.9 #109

Merged
merged 21 commits into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,25 @@ on:

jobs:
Test:
runs-on: ubuntu-latest
name: Run unit tests and examples
strategy:
fail-fast: false
Copy link
Member Author

@gouzil gouzil Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果任何矩阵作业失败,GitHub 将不会取消所有正在进行的作业, 说明链接

matrix:
os: [ubuntu-latest]
python-version: ['3.8', '3.9']
runs-on: ${{ matrix.os }}
name: ${{ matrix.os }} - python ${{ matrix.python-version }}
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Install python
uses: actions/setup-python@v4
with:
# Currently, we only support Python 3.8
python-version: '3.8'
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
wget https://paddle-ci.gz.bcebos.com/dy2st/fallback/linux-whl/paddlepaddle_gpu-0.0.0-cp38-cp38-linux_x86_64.whl
pip install paddlepaddle_gpu-0.0.0-cp38-cp38-linux_x86_64.whl
python -m pip install paddlepaddle==0.0.0 -f https://www.paddlepaddle.org.cn/whl/linux/cpu-mkl/develop.html
pip install -r requirements.txt

- name: Run unit tests
Expand Down
40 changes: 39 additions & 1 deletion symbolic_trace/opcode_translator/executor/opcode_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,6 @@ def build_map(

def BUILD_MAP(self, instr):
map_size = instr.arg
built_map = {}
assert map_size * 2 <= len(
self._stack
), f"OpExecutor want BUILD_MAP with size {map_size} * 2, but current stack do not have enough elems."
Expand Down Expand Up @@ -691,6 +690,16 @@ def COMPARE_OP(self, instr):
f"{instr} is not support. may be not a supported compare op."
)

def IS_OP(self, instr):
# It will only be 0 or 1
assert instr.argval == 0 or instr.argval == 1
left = self.pop()
right = self.pop()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里pop的顺序是不是有点问题?先pop出来的应该是right元素吧

Copy link
Member

@SigureMo SigureMo Jun 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gouzil 编写字节码模拟执行代码时候可以参考 cpython 的 ceval.c,我们在实现的时候也是优先参考和对齐它的:

https://github.com/python/cpython/blob/3.9/Python/ceval.c#L3019-L3031

(虽然 IS_OP 左右应该没太大问题,但最好还是对齐比较好)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是的是的,已修复,这个跟COMPARE_OP其实还是挺像的

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IS_OP 应该是 3.9 版本从 COMPARE_OP 里抽离出来的部分操作,3.8 的 is/is not 是在 COMPARE_OP 里的

if instr.argval == 0:
self.push(SUPPORT_COMPARE_OP["is"](left, right))
else:
self.push(SUPPORT_COMPARE_OP["is not"](left, right))

def MAKE_FUNCTION(self, instr):
fn_name = self.pop()
codeobj = self.pop()
Expand Down Expand Up @@ -925,6 +934,35 @@ def FORMAT_VALUE(self, instr):
else:
raise UnsupportError(f"Do not support format {type(value)} now")

# NOTE: This operation will generate SideEffects, and the mechanism has not been completed yet
def DICT_UPDATE(self, instr):
dict_value = self.pop()
assert instr.argval > 0
retval = {}
# new data
dict.update(retval, dict_value.get_wrapped_items())
# raw data
dict.update(retval, self._stack[-instr.arg].get_wrapped_items())
self._stack[-instr.arg] = DictVariable(
retval, self._graph, DummyTracker(dict_value)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

返回的 DictVariable 与原来的是不共享数据的吗?这是符合预期的吗?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还是不建议在 OpCodeExecutor 里直接操作 value,这也做不到可复用,比如用户调用了 d1.update(d2),仍然需要重新写一遍相同的逻辑,list.extend 有同样的问题


# Like DICT_UPDATE but raises an exception for duplicate keys.
DICT_MERGE = DICT_UPDATE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but raises an exception for duplicate keys.

这一点是如何体现的呢?另外新增的字节码也应该增加单测来确保正确性

Copy link
Member Author

@gouzil gouzil Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DICT_MERGE文档, 这个test_10_build_unpack就会测试到,以及paddle/nn/layer/layers下的_build_once也会使用到

测试值为{"a": a, "b": b}, {"a": c, "b": d}

按照之前的想法如果key重复会直接被CALL_FUNCTION_EX拦截, 这样其实是不太合理的

  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_executor.py", line 306, in run
    is_stop = self.step(cur_instr)
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_executor.py", line 314, in step
    return getattr(self, instr.opname)(instr)  # run single step.
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_executor.py", line 669, in CALL_FUNCTION_EX
    ret = fn(*args, **kwargs)
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/variables.py", line 718, in __call__
    return self.call_function(*args, **kwargs)
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/variables.py", line 795, in call_function
    inline_executor = OpcodeInlineExecutor(self, *args, **kwargs)
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_inline_executor.py", line 47, in __init__
    self._prepare_locals(*args, **kwargs)
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_inline_executor.py", line 54, in _prepare_locals
    bound_args = sig.bind(*args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/inspect.py", line 3045, in bind
    return self._bind(args, kwargs)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/inspect.py", line 3015, in _bind
    raise TypeError('missing a required argument: {arg!r}'. \
TypeError: missing a required argument: 'c'

已修复为:

  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_executor.py", line 306, in run
    is_stop = self.step(cur_instr)
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_executor.py", line 314, in step
    return getattr(self, instr.opname)(instr)  # run single step.
  File "/Users/gouzi/Documents/git/paddle-symbolic-trace/symbolic_trace/opcode_translator/executor/opcode_executor.py", line 948, in DICT_MERGE
    assert result == None, f"Repeat key: {key}"
AssertionError: Repeat key: a

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里报 InnerError 吧,我们之后会统一规范下报错体系书写方式 #114

另外,报错信息和运行时错误保持一致:

TypeError: __main__.foo() got multiple values for keyword argument 'a'

TypeError: __main__.foo() 这部分我们可以暂时先不考虑,后面的报错信息保持一致吧

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

其余 LGTM

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


def LIST_EXTEND(self, instr):
list_value = self.pop()
assert instr.argval > 0
list.extend(self._stack[-instr.arg].value, list_value)

def LIST_TO_TUPLE(self, instr):
list_value = self.pop()
self.push(
TupleVariable(
list_value.value, self._graph, DummyTracker([instr.argval])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里同样可以 get_wrapped_items 一下确保 Tracker 保留

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

)
)

def RETURN_VALUE(self, instr):
assert (
len(self._stack) == 1
Expand Down
3 changes: 3 additions & 0 deletions symbolic_trace/utils/paddle_api_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ def get_paddle_api():
for fn_name in getattr(module, "__all__", []):
fn = getattr(module, fn_name)
if inspect.isfunction(fn):
# NOTE: Maybe add a whitelist list
if fn.__name__ == "in_dygraph_mode":
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同步一下最新的 develop,#108 已修复

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

paddle_api_list.append(fn)
return list(set(paddle_api_list))

Expand Down
12 changes: 11 additions & 1 deletion tests/run_all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
export PYTHONPATH=$PYTHONPATH:../
export STRICT_MODE=1

failed_tests=()

for file in ./test_*.py; do
# 检查文件是否为 Python 文件
if [ -f "$file" ]; then
Expand All @@ -10,7 +12,15 @@ for file in ./test_*.py; do
python "$file"
if [ $? -ne 0 ]; then
echo "run $file failed"
exit 1
failed_tests+=("$file")
fi
fi
done

if [ ${#failed_tests[@]} -ne 0 ]; then
echo "failed tests file:"
for failed_test in "${failed_tests[@]}"; do
echo "$failed_test"
done
exit 1
fi
4 changes: 4 additions & 0 deletions tests/test_10_build_unpack.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
# BUILD_TUPLE_UNPACK_WITH_CALL (new)
# CALL_FUNCTION_EX (new)
# BUILD_MAP_UNPACK (new)
# LIST_EXTEND (new)
# LIST_TO_TUPLE (new)
# DICT_UPDATE (new)
# DICT_MERGE (new)

from __future__ import annotations

Expand Down