aim icon indicating copy to clipboard operation
aim copied to clipboard

[feat] Add Windows support

Open 2-5 opened this issue 3 years ago • 19 comments

Added support for Windows build.

With the associated aimrocks PR, all the Cython extensions are now building and I can import the aim package on Windows.

(venv) X:\Clone\aim-windows\aim>aim version
Aim v3.15.1

I'm now trying to get the tests running.

I will further commit to this PR so please hold on merging it.

2-5 avatar Dec 08 '22 15:12 2-5

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 08 '22 15:12 CLAassistant

I managed to get everything up and running - created an aim repo, did a test run, use the web-ui to see it:

aim_1

aim_2

aim_3

One issue I encountered: after about 2 minutes of aim up running these errors start appearing in the terminal:

aim_4

2-5 avatar Dec 08 '22 16:12 2-5

Hey @2-5! Huge thanks for the effort. Could you please push the latest changes as well? I'll build it on my local windows machine and try to investigate the issue. As it's not clear from the logs what's causing the indexing issues. Also we have a lot of hard-coded '/'s in the path operations, will check those as well :)

mihran113 avatar Dec 08 '22 16:12 mihran113

Everything is pushed.

But for now you need to manually mess around with the .whl files - build both aim and aimrocks with these PR steps, but then manually take the built .whl files and install them in a new venv:

pip aim-3.15.1-cp310-cp310-win_amd64.whl
pip aimrocks-0.2.1-cp310-cp310-win_amd64.whl

Since aimrocks for Windows is not on PyPI yet, I used this requirements.txt to install all the aim dependencies in the same venv. Basically copied everything from the Linux wheel metadata on PyPI (aim-3.15.1-cp310-cp310-manylinux_2_24_x86_64/aim-3.15.1.dist-info/METADATA:Requires-Dist)

aim-ui==3.15.1
aimrecords==0.0.7
# aimrocks==0.2.1
cachetools>=4.0.0
click>=7.0
cryptography>=3.0
filelock>=3.3.0
numpy>=1.12.0
protobuf>=3.11.0
psutil>=5.6.7
py3nvml>=0.2.5
RestrictedPython>=5.1
tqdm>=4.20.0
aiofiles>=0.5.0
alembic>=1.4.0
fastapi<0.68.0,>=0.65.0
jinja2>=2.10.0
pytz>=2019.1
SQLAlchemy>=1.4.1
uvicorn>=0.12.0
Pillow>=8.0.0
protobuf<4.0.0,>=3.9.2
packaging>=15.0
python-dateutil
requests
grpcio>=1.42.0
async-exit-stack>=1.0.0; python_version < "3.7"
async-generator>=1.0; python_version < "3.7"

Also we have a lot of hard-coded '/'s in the path operations, will check those as well :)

On the Python side / works just fine on Windows. Not sure about rocksdb. One issue might be if you directly compare paths with / or \ in them, something along this line: '.aim/aim_db' == os.path.join('.aim', 'aim_db')

I see some logger.info in the code where it fails, I tried running aim -v up to enable verbose mode, but it doesn't seem to enable INFO level for logger.

2-5 avatar Dec 08 '22 16:12 2-5

Thanks a lot, I'll have a look. What about logs, it's enabled by adding a --log-level=info argument to the command.

mihran113 avatar Dec 08 '22 17:12 mihran113

Thanks, I've found it too, but it doesn't seem to work. It looks that there are multiple logging levels and formatters in play (uncolored INFO, green INFO and no WARN level tag):

aim_5

2-5 avatar Dec 08 '22 17:12 2-5

I have discovered the issue, aim.storage.locking uses a filelock.UnixFileLock, it needs to use filelock.WindowsFileLock.

aim_6

NotImplementedError has an empty str representation, which is why the error was not visible in the log:

>>> e = NotImplementedError()
>>> str(e)
''

https://github.com/aimhubio/aim/blob/main/aim/storage/locking.py#L158

2-5 avatar Dec 08 '22 17:12 2-5

The aim test suite is now passing. Two tests are being skipped. One of them was because I didn't have dvc installed. I installed it, and it fails, but it doesn't look to be Windows related:

FAILED
tests/integrations/test_dvc_metadata.py::TestDVCIntegration::test_dvc_files_as_run_param -
AttributeError: <module 'aim.sdk.objects.plugins.dvc_metadata' from
'H:\\Clone\\aim\\test\\.pyenv\\lib\\site-packages\\aim\\sdk\\objects\\plugins\\dvc_metadata.py'>
does not have the attribute 'Repo'

One remaining issue with the tests is that cleanup fails - conftest.py:_cleanup_test_repo raises PermissionError when calling shutil.rmtree(path) because the Repo is still open and holds locks on the files (you can't delete locked files on Windows). I tried both calling repo.close() and Repo.rm(path), but it doesn't seem to work.

2-5 avatar Dec 08 '22 21:12 2-5

I have added /aim/docs/source/using/windows.md with full instructions on how to install aim on Windows.

It assumes that these two PRs are merged in.

Until then, you can replace git clone https://github.com/aimhubio/aim.git and git clone https://github.com/aimhubio/aimrocks.git in the scripts with git clone -b windows-build https://github.com/2-5/aim.git and git clone -b windows-build https://github.com/2-5/aimrocks.git.

There are no further commits from me to be done, aim seems to work correctly on my computer, so you can now test these PRs and see if it's possible to merge them.

2-5 avatar Dec 09 '22 13:12 2-5

@2-5 huge thanks! We will review it asap. Also, I guess, we should check if all the paths are passed via Pathlib and make sure no other platform specific inconsistencies exist.

gorarakelyan avatar Dec 09 '22 17:12 gorarakelyan

Any progress on this? Trying to run on top of a Windows stack and running into issues. Would be nice to be able to have this part of the mainline.

cmoski avatar Jun 08 '23 19:06 cmoski

Any progress on this? Trying to run on top of a Windows stack and running into issues. Would be nice to be able to have this part of the mainline.

+1

Yarin-Shitrit avatar Jun 12 '23 05:06 Yarin-Shitrit

@gorarakelyan any chance you can provide input here? I wonder if this could be part of the next patch. Would love to use it!

flaviuvadan avatar Aug 01 '23 13:08 flaviuvadan

@SGevorg or @gorarakelyan bump! Trying to get an estimate of when this might be released given the interest

flaviuvadan avatar Aug 21 '23 03:08 flaviuvadan

I'm looking forward to windows support too!

xquyvu avatar Sep 05 '23 18:09 xquyvu

Looking forward for windows support as well.

srfiorini avatar Dec 07 '23 13:12 srfiorini

This project look so great, it would be really nice to have it working on windows too!

EtienneT avatar May 23 '24 17:05 EtienneT

Why this PR hasn't been approved yet? I've been looking forward to the support of Windows for a long time😫

robocanic avatar Jul 27 '24 07:07 robocanic