Implement get_total_memory for windows

[mert-kurttutan]

Addresses #103

[CLAassistant]

All committers have signed the CLA.

[mert-kurttutan]

I will also add several test comparing against psutil results on windows workflows, with some several scenarios (e.g. multithreaded call to the get_total_memory function)

[mert-kurttutan]

It might be a nitpick, to be super clear, but the terminology used in the comments is free_memory, but we were getting is total memory.
Probably, we should change variable name to total_memory to be consistent.

[noahho]

Could you please just add another comment explaining what this code is for? The name of the class could also reflect the meaning of it in the wider system. Might it even make sense to move this class inside the function to scope it clearly?

[mert-kurttutan]

It might be a nitpick, to be super clear, but the terminology used in the comments is free_memory, but we were getting is total memory. Probably, we should change variable name to total_memory to be consistent.

Just looked at the todo inside that function, this seems not resolved (whether to go for free or total, for example gpu code still uses free memory, cpus use total_memory). The naming of the variable is couple with this TODO, should be dealt with another PR.

[mert-kurttutan]

The another thing to do is to add windows to os-matrix in workflow. Is that OK with u?
Edit: We can run only the test for windows total memory function on these machine if we want

[noahho]

The another thing to do is to add windows to os-matrix in workflow. Is that OK with u?
Edit: We can run only the test for windows total memory function on these machine if we want

Sorry I didn't quite get this. "os-matrix in workflow" - where would this be?

From my side looks good to merge

[mert-kurttutan]

The another thing to do is to add windows to os-matrix in workflow. Is that OK with u?
Edit: We can run only the test for windows total memory function on these machine if we want

Sorry I didn't quite get this. "os-matrix in workflow" - where would this be?

From my side looks good to merge

Sorry meant here: see matrix.os in https://github.com/PriorLabs/TabPFN/blob/main/.github/workflows/pull_request.yml

[noahho]

The another thing to do is to add windows to os-matrix in workflow. Is that OK with u?
Edit: We can run only the test for windows total memory function on these machine if we want

Sorry I didn't quite get this. "os-matrix in workflow" - where would this be?
From my side looks good to merge

Sorry meant here: see matrix.os in https://github.com/PriorLabs/TabPFN/blob/main/.github/workflows/pull_request.yml

@LeoGrin

[LeoGrin]

Thanks @mert-kurttutan for the PR! Yes adding windows to the CI tests would be great, thanks :) I think we can run all tests on windows.

[mert-kurttutan]

Fails due to heredoc (which is valid only in shells for unix).
An alternative approach is to use python file (located under scripts directory) than use it directly instead of heredoc tool. It is easier to read and more portable (unless there is some specific reason to keep using it).

[mert-kurttutan]

Turns out minimal torch version (the setting matrix.dependency-set == 'minimum' && runner.os == 'Windows' in github CI) has dependency on ONNX with version that is buggy on Windows. So, I needed to force the downgrade of ONNX version on that config.

[LeoGrin]

Thanks a lot @mert-kurttutan for the fixes! I think I would rather skip the onnx compilation tests on Windows, as we don't really need them on all OS. Otherwise it looks great :)