Parskatt / RoMa

[CVPR 2024] RoMa: Robust Dense Feature Matching; RoMa is the robust dense feature matcher capable of estimating pixel-dense warps and reliable certainties for almost any image pair.
https://parskatt.github.io/RoMa/
MIT License
434 stars 33 forks source link

Wrong version of pytorch (without gpu) #4

Closed mpizenberg closed 11 months ago

mpizenberg commented 11 months ago

While you are at it, the torch dependency in the requirements.txt does not provide the correction version of pytorch when in a new virgin environment (like a new conda environment). I had to manually install pytorch with the instructions here: https://pytorch.org/get-started/locally/

Parskatt commented 11 months ago

Im not sure there is any solution to this. On linux you have to supply an index url for cpu. On windows you have to supply it for gpu. If there's an easy way to handle special cases I'll be happy to add it. In general the instructions I provide assumes linux on latest stable gpu pytorch.

mpizenberg commented 11 months ago

I think there are modifiers that can enable architecture-specific dependencies. I'll check if I can find the right way to specify it.

mpizenberg commented 11 months ago

So I found the notation for architecture specific deps (; platform_system == 'Windows') but it was not helpful as I couldn’t specify an index url for one specific package only, it applies to all packages. However, it does seem like pytorch is hosting packages for both linux and windows in their indexes. So it feels like it would actually be ok to specify the index for both Linux and Windows.

Here is a requirements.txt that seems to be working for me, and I think it should work for linux too. Could you let me know if you try?

--index-url https://download.pytorch.org/whl/cu117
--extra-index-url https://pypi.python.org/simple
torch
einops
torchvision
opencv-python
kornia
albumentations
loguru
tqdm
matplotlib
h5py
wandb
timm
#xformers # Optional, used for memefficient attention
Parskatt commented 11 months ago

I think this breaks cpu-only installs, not sure though.

Just for reference, what version is installed with the current requirement file, and what is installed with the suggested one?

mpizenberg commented 11 months ago

Well it might break cpu-only installs but the cpu-only pytorch (the one by default with torch from pypi) actually crashes when I ran the demo, so I was thinking cpu-only was not considered anyway. If it is, then it's currently broken (at least on windows).

Parskatt commented 11 months ago

Haha yeah, probably because I didn't test the cpu versions. Might be some hardcoded .cuda calls remaining, otherwise there should be no major problem getting it on cpu.

mpizenberg commented 11 months ago

Ok, I'll let you decide what you want to do with this issue. Might be as simple as just adding a note on the readme for windows user if you prefer not touching the requirements.txt.

Parskatt commented 11 months ago

I think I'll keep it as-is for now, but might add a disclaimer, and will fix the cpu version.