-
Notifications
You must be signed in to change notification settings - Fork 67
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
Windows OS not supported? #52
Comments
Hi @internetscooter, thank you for the report. We did not intend to exclude Windows OS. While Habitat is not available on Windows, Monty should be available. Does the following # This file may be used to create an environment using:
#
# ## Miniconda or Anaconda
# $ conda env create --file environment_win.yml
# If you are using the zsh shell, run:
# $ conda init zsh
# Or, if you are using a different shell, run:
# $ conda init
# After init, if you do not want conda to change your global shell when
# you open a new terminal, run:
# $ conda config --set auto_activate_base false
# Finally, activate the environment with:
# $ conda activate tbp.monty
#
# platform: win
name: tbp.monty
channels:
- pytorch
- pyg
- defaults
- conda-forge
dependencies:
- python=3.8
- cmake>=3.14.0
- pyg::pyg
- wget
- pytorch::pytorch=1.11.0
- conda-forge::quaternion=2023.0.3 # later versions missing np.long
- pytorch::torchvision
- pip
- pip:
- -e .[dev] For example, the Monty Meets World benchmarks are intended to be independent of Habitat: https://thousandbrainsproject.readme.io/docs/benchmark-experiments#monty-meets-world. (A documentation update is pending in #57, which shares instructions for downloading the data used in the benchmarks.) Unfortunately, once you pointed out the problem, it became apparent that Monty is accidentally coupled to Habitat in some of the motor policies:
quat_rotate_vector and common.quat_to_magnum are used, and can likely be replaced by non-Habitat implementations.
|
Would yo be happy including SciPy? |
@vkakerbeck, @nielsleadholm any thoughts on habitat_sim.utils replacements and @internetscooter 's suggestion above? |
I agree, scipy's Rotation function seems the most straight forward function to replace the use of Line 26 in 70d610a
For the quaternion <-> magnum conversions ( quat_to_magnum and quat_from_magnum ) it seems like we could just call whatever habitat implements in the respective functions instead (https://github.com/facebookresearch/habitat-sim/blob/d7da04897f234727813aee3368d7e44e0f08c50e/src_python/habitat_sim/utils/common.py#L48) since its just a straight forward call to magnum functionality (mn.Quaternion(quat.imag, quat.real) )
|
Thanks @vkakerbeck! After you mentioned calling tbp.monty/src/tbp/monty/frameworks/models/motor_policies.py Lines 1212 to 1236 in 0d33c9c
|
I'm kind of confused why we convert here to magnum anyways. We could just invert the quaternion directly... @nielsleadholm any recollection why we did that? |
I dug into the pre-open source code history, and based on that context (PR 201), the magnum conversion seems incidental and not done for a specific reason. |
As reported in https://thousandbrains.discourse.group/t/implimentation-problems-on-a-raspberry-pi/187/7, tests are another source of coupling. Following the default instructions and running |
Okay, so it sounds like for getting Monty running without habitat we can use Making the unit tests independent of habitat will be a lot more involved 😬 This can be done in a second step later on. |
@tristanls it still failed on the wget. I updated that to python-wget and that solved the errors Edit: However pytest fails with |
Installing ` tests/unit/evidence_sdr_lm_test.py::EvidenceSDRIntegrationTest::test_can_generate_reasonable_sdrs -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html |
Just adding my voice @tristanls @vkakerbeck that I agree the use of magnum looks like it was incidental rather than required, and Scipy should work well as an alternative. |
I'm looking at |
@internetscooter Thanks for raising bug, I can replicate your result (1 less warning). ================================ 168 passed, 1 warning, 14 errors in 197.31s (0:03:17) ================================ @tristanls Will the modified environment_win.yml be committed into git repo? |
@leemkuny, yes, some version of environment.yml for Windows users should go into the repository so that installation path is more straightforward, but I'm not sure what the final version of it would be yet. Do you have a version that works without modification? |
I removed the HabitatSim dependency from With coupling removed in
when attempting
|
Hi, @internetscooter, did you try on WSL? It's pretty seamless on WSL for windows users. I had to do very little updates to get the tests running. But it may not be performant for obvious reasons. |
Not yet - would it not be performant due to CUDA? This might be resolved with Enable NVIDIA CUDA on WSL. If I am likely to end up with a fully functional performant set-up, I am happy to put in the work and document the whole process while doing it :) |
Describe the bug
Hi TBP,
I think Windows OS is not supported due to facebookresearch/habitat-sim#1570 - if this is the case there should be a note on this in the code/docs?
Below is the complete message. If the above is the case I will see what workaround I can come up with.
PS C:\code\tbp.monty> conda env create C:\Users\paulm\anaconda3\Lib\argparse.py:2006: FutureWarning:
remote_definitionis deprecated and will be removed in 25.9. Use
conda env create --file=URL` instead.action(self, namespace, argument_values, option_string)
Channels:
Platform: win-64
Collecting package metadata (repodata.json): done
Solving environment: failed
PackagesNotFoundError: The following packages are not available from current channels:
Current channels:
To search for alternate channels that may provide the conda package you're
looking for, navigate to
and use the search bar at the top of the page.`
The text was updated successfully, but these errors were encountered: