-
Notifications
You must be signed in to change notification settings - Fork 35
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.
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
VS Build tools required to install #7
Comments
Hi @andreped, Thanks for pinging me. I think this will be great. I'm sure @masadcv and @SachidanandAlle can provide more information about the use of simpleCRF in MONAI Label. |
Thanks @diazandr3s for tagging me here. As @andreped mentioned, SimpleCRF requires compiling, i.e. It would be good to have pre-compiled binaries for this if possible, however I am unsure how much work this would be as compilation is required for all systems and for each python version. Some functions depend on numpy for compilation – I am unsure how this can be handled to make pre-compiled binaries that work across platforms but also with different versions of numpy. Lets wait for someone from HiLab to comment on this. cc: @taigw |
@masadcv I will make an attempt and we will see how easy it is :P Will let you know when the workflow from making pre-compiled binaries is running and I can make a PR and we can co-develop from there? I can setup builds for various Python versions. However, from the setup.py, I see that there is no limitations on Python version used. Is that so? Have you tested installing SimpleCRF on different versions? |
I have gotten quite far. What remains is setting up such that python limited api is used and creating a wheel for each operating system. However, I see that classifiers and python_requires in setup.py are contradicting. Do you only support Python 3.6 and above, or do you want to support Python 2 as well? See here. If only Python >=3.6, then it is easy. See my fork for my proposed workflow: It is still a work in progress, but at least the wheels are produced. As the software include C++ code, I will have to make a wheel for each operating system. I can set that up now. |
We don't want to go back.. we don't want to worry about python 2.x |
Understandable. Will let you know when I have the wheels ready for you to test. Also, I played a lot around with getting the demos to run on the cloud, but it failed on Windows and macOS. On the latter it failed due to some graphics-related issue, which makes sense as I believe it runs headless? Also OpenGL is not supported on macOS any longer, so it complained about that as well. For Windows it froze on plt.show(). Perhaps having a simple way to neglect that if not possible would be better. I tested the maxflow demo. |
Forward-compatible python wheels with abi3 for windows, ubuntu, and macos are now being created: These wheels are compatible with Python >=3.6 as discussed. It is also possible to push these directly to PyPI in the workflow. However, AFAIK this would require permissions to your PyPI repo which I doubt I have nor need. A separate workflow to release could be a good idea, for instance to only make the release when a new tag is made. Feel free to test the workflow and produced binaries on different operating systems, different python versions, and different setups. Or even better, make unit tests which can be deployed as workflows instead. The final part has to be done by the developing team, and then I guess the MONAI-Label team and @diazandr3s could test if the new pre-compiled binaries can be used directly when installing through pip. Especially relevant for windows which previously required VS and Visual C++ build tools to be able to install SimpleCRF. |
@masadcv can we try and test the precompiled version of crf.. this will bring more convenience for MONAILabel users while installing the package |
Definitely. With GitHub Actions these are compiled on the fly and made available as artifacts. For instance for ubuntu, see here, under Artifacts there should be a file called "Python wheel". Download and extract and you should have the python wheel. Similar wheels are produced for all operating systems - see here to access wheels for different builds. EDIT: Might seem like there is a minor DLL issue. When testing on my personal, local windows desktop, I got |
@SachidanandAlle sure, I will do some tests locally. I only have access to Ubuntu machine at the moment. Can someone help verify Windows and Mac OS MONAI Label setups for this? In the meantime I will try to find a Windows machine to verify this. Nice work @andreped !! What's the best way to access the compiled wheels? Would something like |
You cannot do the proposed "pip install" solution as this will not download the binaries, but rather run the setup.py install as you previously did for SimpleCRF, and then we are back to square one. I can publish the binaries as releases on my fork and you can access them there if you'd like. Otherwise, the binaries are found as Artifacts for each build. Currently, there seems to be a bug, so no point publishing as a release just yet. When the solution is working as it should, the ideal solution would be to put these in the releases on PyPI, but as of now, we should verify that this actually works first :] |
Hi, so I am not a technical programmer. I did install Microsoft
C++2015-2019 and Visual studio installer. Not sure what I need to do. I
have with Andres through teams in which we shared the workstation screen to
help me set it up. I wonder if this could be a way to help me set up the
system. The only thing is that I don't have a camera and a mike on the
workstation, so I talked to Andres via the laptop.
Alternatively, if you don't mind let me know what should I install in
preparation or to be able to work with Andres to solve this
thanks Ron
…On Mon, Jan 31, 2022 at 8:25 PM Andres Diaz-Pinto ***@***.***> wrote:
Hi @andreped <https://github.com/andreped>,
Thanks for pinging me. I think this will be great.
Recently, a Windows user ***@***.*** <https://github.com/ralkalay>) trying
to install MONAI Label on his computer had a VS-related problem. See this
issue: Project-MONAI/MONAILabel#363
<Project-MONAI/MONAILabel#363>
I'm sure @masadcv <https://github.com/masadcv> and @SachidanandAlle
<https://github.com/SachidanandAlle> can provide more information about
the use of simpleCRF in MONAI Label.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALXEEN5OAZY4Q7DZMQBOEVDUY4Y6ZANCNFSM5NGANCVQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ralkalay the whole idea with this solution is that you should not need VS and C++ build tools to use SimpleCRF and AFAIK MONAI-Label for that manner. Simply Are there any other reasons why VS and C++ build tools would be needed for just using the binary of MONAI-Label, @diazandr3s ? I am not talking about building anything just using the plugin with slicer. |
Andres and I were installing via pip install -r requirements.txt
should I do it differently? thanks
On Tue, Feb 1, 2022 at 10:27 AM André Pedersen ***@***.***> wrote:
@ralkalay <https://github.com/ralkalay> the whole idea with this solution
is that you should not need VS and C++ build tools to use SimpleCRF and
AFAIK MONAI-Label for that manner. Simply pip install SimpleCRF and pip
install monailabel
Are there any other reasons why VS and C++ build tools would be needed for
just using the binary of MONAI-Label, @diazandr3s
<https://github.com/diazandr3s> ? I am not talking about building
anything just using the plugin with slicer.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALXEEN5FLEIIHLB7NOBHR4TUY73VLANCNFSM5NGANCVQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
aiofiles==0.6.0
fastapi==0.65.2
monai[nibabel, skimage, pillow, tensorboard, gdown, ignite, torchvision, itk, tqdm, lmdb, psutil, openslide]==0.8.*
pyyaml==5.4.1
python-multipart==0.0.5
requests-toolbelt==0.9.1
uvicorn==0.13.4
watchdog==2.0.2
pydantic==1.8.2
python-dotenv==0.17.1
filelock==3.0.12
httpx==0.18.2
dicomweb-client==0.52.0
pydicom==2.1.2
pydicom-seg==0.2.3
pynetdicom==1.5.7
expiringdict==1.2.1
schedule==1.1.0
timeloop==1.0.2
simplecrf==0.2.1.1
einops==0.3.2
|
@ralkalay well currently my binaries are not published as part of a release on PyPI, so simply doing However, when we get this properly working, then you can continue to do I am experiencing some DLL issues on two machines. One of which seem to be related to the abi3 solution I made which may not seem to be working perfectly yet. |
Andre thanks
Looking forward to the solution. It took 40 years in the desert before
Israel was allowed into the promised land. Patience is a built-in feature,
Ron
…On Tue, Feb 1, 2022 at 10:51 AM André Pedersen ***@***.***> wrote:
@ralkalay <https://github.com/ralkalay> well currently my binaries are
not published as part of a release on PyPI, so simply doing pip install
SimpleCRF or placing SimpleCRF in the requirements.txt file will not work.
However, when we get this properly working, then you can continue to do pip
install -r requirements.txt. But it is not ready yet. So if you are
experimenting with MONAI-Label using my fix it is too soon. We need to
check that this actually works first and solves the issue.
I am experiencing some DLL issues on two machines. One of which seem to be
related to the abi3 solution I made which may not seem to be working
perfectly *yet*.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALXEENZN3XRK3FJFLVY5WPTUY76PTANCNFSM5NGANCVQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@masadcv OK, I have a workin solution now. At least when testing it on my machines with different python versions on both Windows and Ubuntu Linux it installs and running Precompiled versions have been made accessible here: If you experience any issues, please, let me know and I can try to fix it. You are free to test it now @ralkalay, but to install you have to run the lines (example for Ubuntu Linux and Python 3.7):
When this is properly working (at least verified), you can merge my PR #8 and I could assist you in setting up the workflow, such that it releases on PyPI directly, or at least as a Release (tag), which avoids all this download Artifacts manually nonsense. I gave up on the abi3 solution for EDIT: Almost forgot. @diazandr3s the numpy version that is used in SimpleCRF (and the final one that is used in monailabel) does not work out of the box on Windows. I think the warning I got was |
Andre
Thanks, I will work with Andres to see if this solves my issue. Regards,
Ron
…On Tue, Feb 1, 2022 at 11:51 AM André Pedersen ***@***.***> wrote:
@masadcv <https://github.com/masadcv> OK, I have a workin solution now.
At least when testing it on my machines with different python versions on
both Windows and Ubuntu Linux it installs and import denseCRF, maxflow
prompts no warning.
Precompiled versions have been made accessible here:
https://github.com/andreped/SimpleCRF/releases/tag/v0.2.1.1-rc1
If you experience any issues, please, let me know and I can try to fix it.
When this is properly working (at least verified), you can merge my PR and
I could assist you in setting up the workflow, such that it releases on
PyPI directly, or at least as a Release (tag), which avoids all this
download Artifacts manually nonsense.
I gave up on the abi3 solution for one wheel to rule them all, and now I
am building a binary for each python version individually. This is quite
common to do, but I know that it is possible to have one wheel that is
forward compatible. However, it turned out to be not so easy.
—
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALXEEN6SEDN54JC2ZCVJVMTUZAFSTANCNFSM5NGANCVQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks for this @andreped. I think the issue @ralkalay brings to this conversation is related to VS version (Project-MONAI/MONAILabel#363 (comment)). Thanks again!! |
@diazandr3s All this VS and C++ build tools stuff is very annoying and not easy to understand at times. If we are able to get around that, it would make installing MONAI Label a lot easier on Windows. Just so that we understand each other, if you use my binaries for SimpleCRF you do not need VS nor its C++ build tools to install and use MONAI Label. You are already using a precompiled version that can be installed directly. Therefore, the issue you mentioned would be solved by simply using my binaries instead of the old install method. When the binaries are verified to be working properly, the SimpleCRF team can add these to the release, which would mean that during installation by Let me know how the integration and tests go, when you get that far :] |
Thanks for releasing the pip packages @andreped . Line 6 in e9cc27a
The easiest way to verify if all works is to set everything up with monai label, run one of the provided apps and try updating with scribbles (e.g. like this demo: https://www.youtube.com/watch?v=kVGf5QQxSfc). Alternatively, you can run the demo2d.py and demo3d.py scripts within SimpleCRF repo to verify everything. |
I agree. I would suggest doing that. I don't have the time currently, as I have to participate in other projects for the remained of the week, but if you are involved in developing and/or testing monai label, perhaps you could test it, @masadcv ? I can setup some unit tests to test the installed version on the cloud, for instance using the demos provided. However, I have tested the demo scripts and they work well, but they cannot be run for all operating systems on the cloud (discussed here). The reason is simply that the current scripts require a screen to show the results (or at least thats what I think is the reason). This worked flawlessly on windows, but had issues both on Ubuntu and macOS, if I recall correctly. EDIT: The demo works on Ubuntu but not Windows and macOS. Modifying the demos now such that I can use them for unit testing on the cloud. |
Just made the necessary modifications to the demos, such that I could use them in the workflows. Added units tests for all three demos, which is now running (and working) for all operating systems and Python versions 3.6-3.9 (for build history, see here). SimpleITK, which is used in the demos, does not currently have a precompiled version for Python 3.10, and thus I am avoiding building and testing for that currently. However, I guess SimpleCRF should work, but I am unable to test that on the cloud using the demos. I could introduce an if statement to avoid running the unit tests for Python 3.10, but lets avoid that for now. Even though they work well on the cloud, they were built on the same machine they are being tested on, and therefore it would be better if someone could install the wheels on new machines with different setups to see whether the wheels are working properly in their pipelines. Then we would know for certain. But right now I am quite confident that the solution is working. EDIT: Now all wheels are built from a single workflow, which means that all wheels are added to the same "Python wheel.zip". For instance, see here. |
This will be great! |
@diazandr3s it is ready to be tested with monailabel. Let me know how it goes :] |
Many thanks, @andreped. Will try on @ralkalay's computer and see how that goes. How I should install it? Please let me know |
Thanks for this @andreped ! Unfortunately, this week has been terribly busy with all the research deadlines. I will try to set things up next week and update once I have this tested. There is an additional error reported for SimpleCRF on Conda here: Project-MONAI/MONAILabel#605 |
@diazandr3s As the binaries are located at: You can simply install by running:
Given that you have Python 3.8 and using a Linux machine. For other setups, you can simply adjust the values of the URL. Currently Python 3.6-3.10 are supported for the operating systems: Win10, Ubuntu Linux, and macOSX. See the release tag for more info on how the paths are structured for the different setups. When the binaries have been added to the actual release for SimpleCRF on PyPI, you can install as you normally would by
I have not tried to install (or test) the binaries using conda. I never use conda myself. Not really fond of it tbh. However, doesn't hurt to try. Probably works. I know lots of people have issues installing and using libraries using conda. My suggestion is use avoid conda and use pip instead and all your pain and suffering goes away. Also, yeah, what you are observing with conda might just be a linker issue. Might be that it is solved using the precompiled versions. No idea really. Just test when you have the time and let me know how it went, @masadcv :] Otherwise, I could setup some tests for conda in GitHub Actions to see for myself, but I'd rather not ;) |
@andreped I see multiple versions of binaries for windows (due to CPP version).. later when we do pip install SimpleCRF.. will that take care of selecting the suitable version automatically? Because ultimately the dependency in requirements.txt will be I see you have already answered my question above.. |
@SachidanandAlle you cannot have If you want to install SimpleCRF, you would have to install it separately. So simply do:
and modify the latter to fit your setup (Python version and OS). And of course, remove EDIT: The reason why this does not happen automatically, is because when you It is possible to setup a similar PyPI-esque solution to install binaries outside PyPI, but I have not done that before. Maybe this blog post is interest, if you were to attempt to setup this install yourself: |
I meant |
Oh, not sure this works on Windows. In general, softwares are both backward and forward compatible on Windows. Hence, if a binary is built on Win10 it should work on Win11, but I have no way of testing that myself. Also, that was good that you mentioned, because we should verify whether this works on newer version of macOS (should be forward compatible). For Linux Ubuntu I might have to build a manylinux version instead. But let me know how it goes first.
As I am only a contributer, not the developer of SimpleCRF nor can modify the SimpleCRF PyPI repo, it is the SimpleCRF team that has to take this further. Hopefully they do this soon, as there are a lot of people interested in the fix. |
Have verified for following
Looks Good to me.. @masadcv can you also double-check the functionality and if all is ok then along with @andreped approach the SimpleCRF team to make this as official release. |
Hi @andreped, Thanks for driving this. Thanks again!! |
Hi @andreped ,
I dont have access to Windows setup, but we have multiple confirmations for this above. I have a couple of comments on the PR:
@taigw please have a look at the PR #8 which provides workflow for pre-compiled binaries for SimeplCRF for a range of different machines. We may additional be required to release this on PyPi. This improves the reach of this library to machines which may have problems in compiling it (we have seen a number of user report this on Windows machines, see: Project-MONAI/MONAILabel#363 for example). Many thanks! |
Luckily, adding support for more Python versions is simple. However, it is likely not worth the effort to add support for too old Python versions (what insane man still uses python 2.7? python 2 is even no longer maintained!), especially not Python 2. But if that is of interest, I can modify the setup.py and workflow scripts to produce more wheels. Currently, I have only made wheels for Python >= 3.6, but I can add more, if you'd like. Just come with requirements, @taigw :] |
Until this fix has been added to the main PyPI project (SimpleCRF), after discussion with @SachidanandAlle in the PR #8, I have created a separate temporary PyPI project which includes these binaries at: Therefore to use my fix, simply install SimpleCRF by: This should solve your issue relevant for MONAI Label, @diazandr3s and @masadcv. |
@andreped many thanks! I will give it a try in coming days... If all looks good, maybe we can have a PR for MONAI Label to switch to using these. |
@masadcv Yes, definitely. I could make that PR, if you'd like. About time I started contributing more :P Note that I also bumped version to v0.2.1.2 from v0.1.2.1 to avoid the issue of people assuming these wheels are identical. The README of this project also contains a disclaimer. |
Yes, please go for it and let's take this and any other MONAI label discussion there :) |
@masadcv a PR has now been made (see Project-MONAI/MONAILabel#725). I can mention that a PR has been made in this issue Project-MONAI/MONAILabel#719. |
I see only so files.. python modules for maxflow and denseCRF are missing |
For clarity here as well, I made a comment about the SimpleCRF-binaries at #8 (comment). |
Strange... Was this working before with the previous wheels? Also, which operating system were you testing on? I could setup some unit tests using these wheels to test this further. |
now we have new problem.. license.. it's not compatible for monai+ @andreped ^^ |
@SachidanandAlle does this mean that the wheels work as they should, given that the correct numpy version is installed? Issue seemed to be resolved in this thread: I could update the SimpleCRF installation to use a newer version of numpy. |
The corresponding PR has now been merged with master: f75618b which means that precompiled wheels will be generated for each commit. For instance, see here: However, a new release should be made which uses the precompiled wheels instead. Until that is done, you can install SimpleCRF by:
|
This is probably more of a feature request than a bug.
Whenever I wish to install this solution on Windows it requires that I have VS build tools installed as it seems like it needs to build maxflow. This becomes a challenge as some of the users of a software I am using find it challenging to install this.
What I was thinking is why this is not just built on the cloud, for instance using GitHub Actions, and then when installing you are just catching the pre-built wheel and using that to install. I guess this would be of interest to the MONAI-Label team (@diazandr3s) as well.
I can assist in setting up the Actions workflow and submit a PR, if that would be of interest.
I could attempt to setup builds for the operating systems: Windows, macOSX, and Ubuntu Linux.
Shall I make a try?
The text was updated successfully, but these errors were encountered: