Skip to content

Add math test #555

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.

Sign up for GitHub

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

Closed
yiranwu0 wants to merge 18 commits into from
Closed

Add math test #555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d39e293
update
yiranwu0 Nov 5, 2023
84c303c
update
yiranwu0 Nov 5, 2023
a11831a
update
yiranwu0 Nov 5, 2023
b759541
update
yiranwu0 Nov 5, 2023
6e34f78
update
yiranwu0 Nov 5, 2023
69b253a
update
yiranwu0 Nov 5, 2023
44bf6fa
update
yiranwu0 Nov 5, 2023
7e4cbb7
update
yiranwu0 Nov 5, 2023
05e4cf1
update
yiranwu0 Nov 5, 2023
63c6320
update
yiranwu0 Nov 5, 2023
6e0324b
update
yiranwu0 Nov 5, 2023
c80e366
update
yiranwu0 Nov 5, 2023
3210eee
update
yiranwu0 Nov 5, 2023
86ae46d
check when dependency fail
yiranwu0 Nov 5, 2023
189a4d3
revision
yiranwu0 Nov 5, 2023
c0dfd3d
revise
yiranwu0 Nov 5, 2023
9396960
Merge branch 'main' into mathtest
yiranwu0 Nov 7, 2023
9735537
Merge remote-tracking branch 'origin/main' into mathtest
yiranwu0 Nov 11, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
- name: Coverage
if: matrix.python-version == '3.10'
run: |
pip install -e .[mathchat,test]
pip install -e .[test]
pip uninstall -y openai
coverage run -a -m pytest test
coverage xml
Expand Down
25 changes: 22 additions & 3 deletions .github/workflows/contrib-openai.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:
- 'setup.py'

jobs:
RetrieveChatTest:
OpenAI4ContribTests:
Copy link
Contributor

@sonichi sonichi Nov 5, 2023

Choose a reason for hiding this comment

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

One problem with this approach is that the previous installed dependencies will remain in the later steps. That makes the test environment not clean for later steps. Is it possible to reset the environment for later steps?

Copy link
Contributor Author

@yiranwu0 yiranwu0 Nov 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I haven't found any good way to clear up the environment. A possible solution is to use virtual env, but different os needs different syntax, which makes it complex. The other way is just putting them in different jobs.

I guess there are two problems to consider here:

  1. Is it ok to NOT having a clean env? Will there be a case that user does pip install autogen[retrievechat, mathchat], and they expect things just work?
  2. I suggested that separating the jobs would be tedious to look in github actions, which is why I change this in the first place. But we might need to revisit the problem: do you think it is indeed a issue, or we can bear with it?

Copy link
Contributor

@rickyloynd-microsoft rickyloynd-microsoft Nov 5, 2023

Choose a reason for hiding this comment

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

I think it's pretty important for the automatic jobs to test different contributed agents in different environments, each with just its own dependencies, whether this is done through separate virtual envs, different jobs, or some other mechanism. Wish I had more experience in setting up such tests.

Copy link
Contributor Author

@yiranwu0 yiranwu0 Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

coverage run -a -m pytest test/test_retrieve_utils.py test/agentchat/contrib

will test all files in the contrib folder, while only dependencies for Retrievechat is installed. This will result in error in the future if a contrib agent requires other packages to be installed.
https://github.com/microsoft/autogen/blob/fda7a39dd9ede1c115d37c2a454f55b7f22c8193/.github/workflows/contrib-tests.yml#L52C38-L52C38

strategy:
matrix:
os: [ubuntu-latest]
Expand All @@ -37,19 +37,38 @@ jobs:
pip install -e .
python -c "import autogen"
pip install coverage pytest-asyncio
- name: Install packages for test when needed
- name: Install packages for Retrievechat
run: |
pip install docker
pip install qdrant_client[fastembed]
pip install -e .[retrievechat]
- name: Coverage
- name: Coverage Retrievechat
env:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
AZURE_OPENAI_API_KEY: ${{ secrets.AZURE_OPENAI_API_KEY }}
AZURE_OPENAI_API_BASE: ${{ secrets.AZURE_OPENAI_API_BASE }}
OAI_CONFIG_LIST: ${{ secrets.OAI_CONFIG_LIST }}
run: |
coverage run -a -m pytest test/agentchat/contrib/test_retrievechat.py test/agentchat/contrib/test_qdrant_retrievechat.py

- name: Install packages for MathChat
id: install_mathchat
if: ${{ always() }}
run: |
pip install docker
pip install -e .[mathchat]
- name: Coverage MathChat
if: ${{ always() && steps.install_mathchat.outcome == 'success' }}
env:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
AZURE_OPENAI_API_KEY: ${{ secrets.AZURE_OPENAI_API_KEY }}
AZURE_OPENAI_API_BASE: ${{ secrets.AZURE_OPENAI_API_BASE }}
OAI_CONFIG_LIST: ${{ secrets.OAI_CONFIG_LIST }}
run: |
coverage run -a -m pytest test/agentchat/contrib/test_math_user_proxy_agent.py
Copy link
Collaborator

@thinkall thinkall Nov 5, 2023

Choose a reason for hiding this comment

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

Looks like the output of coverage run will overwrite the output of the former step. In the end, only one step of coverage will be uploaded.

Copy link
Collaborator

@thinkall thinkall Nov 5, 2023

Choose a reason for hiding this comment

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

Could we add different names to the output of different steps and combine them at the upload step?

Copy link
Contributor Author

@yiranwu0 yiranwu0 Nov 5, 2023

Choose a reason for hiding this comment

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

The coverage run -a will append outputs to the same file(.coverage), so we are good with this. Not very sure but this is what I find.


- name: convert coverage to xml
run: |
coverage xml
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v3
Expand Down
20 changes: 18 additions & 2 deletions .github/workflows/contrib-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ name: ContribTests

on:
pull_request:
branches: ['main', 'dev/v0.2']
branches: ['main']
paths:
- 'autogen/**'
- 'test/agentchat/contrib/**'
Expand All @@ -17,7 +17,7 @@ concurrency:
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

jobs:
RetrieveChatTest:
ContribTests:
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
Expand All @@ -34,6 +34,7 @@ jobs:
run: |
python -m pip install --upgrade pip wheel
pip install pytest
# RetrieveChat
- name: Install qdrant_client when python-version is 3.10
if: matrix.python-version == '3.10' || matrix.python-version == '3.8'
run: |
Expand All @@ -43,8 +44,23 @@ jobs:
pip install -e .[retrievechat]
pip uninstall -y openai
- name: Test RetrieveChat
if: matrix.python-version != '3.10'
run: |
pytest test/test_retrieve_utils.py test/agentchat/contrib/test_retrievechat.py test/agentchat/contrib/test_qdrant_retrievechat.py

# MathChat
- name: Install packages and dependencies for MathChat
id: install_mathchat
if: ${{ always() }}
run: |
pip install -e .[mathchat]
pip uninstall -y openai
- name: test MathChat
if: ${{ always() && steps.install_mathchat.outcome == 'success' && matrix.python-version != '3.10'}}
run: |
pytest test/agentchat/contrib/test_math_user_proxy_agent.py

# Coverage
- name: Coverage
if: matrix.python-version == '3.10'
run: |
Expand Down
23 changes: 13 additions & 10 deletions test/agentchat/test_math_user_proxy_agent.py → ...hat/contrib/test_math_user_proxy_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,22 @@
_remove_print,
_add_print_to_last_line,
)
from test_assistant_agent import KEY_LOC, OAI_CONFIG_LIST
import os

sys.path.append(os.path.join(os.path.dirname(__file__), ".."))
from test_assistant_agent import KEY_LOC, OAI_CONFIG_LIST # noqa: E402

try:
from openai import OpenAI
import openai

OPENAI_INSTALLED = True
except ImportError:
skip = True
else:
skip = False
OPENAI_INSTALLED = False


@pytest.mark.skipif(
skip or sys.platform in ["darwin", "win32"],
reason="do not run on MacOS or windows",
not OPENAI_INSTALLED or sys.platform in ["darwin", "win32"],
reason="do not run on MacOS or windows or dependency is not installed",
)
def test_math_user_proxy_agent():
from autogen.agentchat.assistant_agent import AssistantAgent
Expand Down Expand Up @@ -119,7 +122,7 @@ def test_generate_prompt():


if __name__ == "__main__":
# test_add_remove_print()
# test_execute_one_python_code()
# test_generate_prompt()
test_add_remove_print()
test_execute_one_python_code()
test_generate_prompt()
test_math_user_proxy_agent()