Add support for Python 3.10 and 3.11 #1937
Conversation
|
hey @simonzhaoms the tests are broken right now, see #1934. The last thing I've been trying to do is to build a docker file and inject it into the RunConfiguration. I think we should first fix the tests, and then we can do the upgrade to python 3.10 and 3.11. Do you think you could help me with it? |
Sure, I can take a look. |
|
hey @simonzhaoms, after the tests are fixed, we should do a PR to main. Do you want to merge this PR before doing the PR to main or after? |
simonzhaoms
changed the title
after, because there are still some problems with this PR to be solved. |
|
@miguelgfierro this PR is still a draft, not ready for review. |
|
After upgrading the dependent packages and the docker images in the commit b71c4ed, it failed because AzureML SDK tried to compile and install an old version of 
It looks like an issue with AzureML SDK and Conda (See Azure Machine Learning SDK installation failing with an exception). And it seems that AzureML SDK sets the upper version for 
I tried setup.py of the commit b71c4ed on my local machine with Python 3.8 3.9, 3.10 and 3.11. All works because there is no dependency on the old version of Possible solutions:
|
| @@ -181,7 +178,20 @@ def create_run_config( | |||
| run_azuremlcompute = RunConfiguration() | |||
| run_azuremlcompute.target = cpu_cluster | |||
| run_azuremlcompute.environment.docker.enabled = True | |||
| run_azuremlcompute.environment.docker.base_image = docker_proc_type | |||
| # See https://learn.microsoft.com/en-us/azure/machine-learning/how-to-train-with-custom-image?view=azureml-api-1#use-a-custom-dockerfile-optional | |||
There was a problem hiding this comment.
@simonzhaoms right now in the actions we are installing run: pip install --quiet "azureml-core>1,<2" "azure-cli>2,<3". What is the azurml sdk that we are installing? Here I see the latest one as 1.51 https://pypi.org/project/azureml-sdk/#history.
Of the 3 options, I think one that is interesting to explore would be Try install everything in the docker file without using Conda. iif we are reducing dependencies. I think 80% of our problems come from dependencies: #1936 So maybe something to reflect on is how can we reduce dependencies and use more standardize and robust software?
There was a problem hiding this comment.
@simonzhaoms right now in the actions we are installing run: pip install --quiet "azureml-core>1,<2" "azure-cli>2,<3". What is the azurml sdk that we are installing? Here I see the latest one as 1.51 https://pypi.org/project/azureml-sdk/#history.
I think this azureml-core is only used for launching the script submit_groupwise_azureml_pytest.py.
And the AzureML SDK I mentioned is used inside the docker image launched inside submit_groupwise_azureml_pytest.py
I think when we use CondaDependencies.add_pip_package("xxx"), AzureML adds the item xxx in a Conda env yaml file maintained by itself, and the AzureML SDK is also an item added by AzureML by default implicitly.
What I don't know is why the AzureML SDK trigger the error now when I try to add support for Python 3.10 and upgrade all dependencies.
There was a problem hiding this comment.
Of the 3 options, I think one that is interesting to explore would be Try install everything in the docker file without using Conda. iif we are reducing dependencies. I think 80% of our problems come from dependencies: #1936 So maybe something to reflect on is how can we reduce dependencies and use more standardize and robust software?
I agree. In addition, if possible, I'd use what GitHub actions and workflows can provide to build the testing pipeline rather than use the AzureML service, because AzureML service is not transparent.
@loomlike The errors can be found at the nightly build on the branch simony-dep-upgrade-20230606: https://github.com/recommenders-team/recommenders/actions/runs/8036183347 But the newest error is the one pasted by Miguel above from running the notebook |
Signed-off-by: Jun Ki Min <[email protected]>
Signed-off-by: Jun Ki Min <[email protected]>
|
Hi @miguelgfierro the gpu machine seems to be in a bad status. Several runs of tests have the same error saying
Could you help to check when you have time? |
Signed-off-by: miguelgfierro <[email protected]>
|
@SimonYansenZhao It seems that error is solved. Now we have some TF errors: https://github.com/recommenders-team/recommenders/actions/runs/8310055489/job/22742112981?pr=1937 |
Signed-off-by: miguelgfierro <[email protected]>
Signed-off-by: miguelgfierro <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Signed-off-by: miguelgfierro <[email protected]>
|
@SimonYansenZhao see this comment: #2071 (comment) |
Signed-off-by: miguelgfierro <[email protected]>
Signed-off-by: Simon Zhao <[email protected]>
Fixing TF to < 2.16
|
@SimonYansenZhao if the unit tests pass, what do you think of merging this PR, and then figure out how to fix the issues we have in the nightly? |
| df = train if validation is None else pd.concat([train, validation]) | ||
| df = df if test is None else pd.concat([df, test]) |
There was a problem hiding this comment.
You can avoid having two concats with this one liner:
df = pd.concat(filter(None, [train, validation, test]))
There was a problem hiding this comment.
hey @daviddavo, can you please create a PR to staging with these changes? I gave you written permissions.
git checkout staging
git branch -b new_branch
.... code changes ...
git commit -asm "you message don't forget the -s to sign the commits"
git push origin new_branch
And then PR.
sounds good @miguelgfierro |
Description
This PR added the support for Python 3.10 and 3.11 with the following changes:
However, this upgrade doesn't fix the following issues:
Related Issues
Checklist:
staging branchand not tomain branch.