I’d like to ask in what measure we could consider getting rid of this
pytest.importorskip(
"habitat_sim",
reason="Habitat Sim optional dependency not installed.",
)
In the scope of habitat-ipc I tried to run the tests with habitat_sim present only in the simulator process, and saw a huge number of tests skipped because of these lines.
Since these skipped tests are not just testing habitat, we lose a lot of test coverage in monty without them. I ran coverage in the main branch with and without habitat, and it went from 79% to 41%, respectively.
Habitat will eventually become permanently absent from the monty repo, but many of these tests will still need to talk to a remote habitat simulator, so the absence of habitat_sim on the python path per se will not mean that the tests should be skipped, which shows that these things are probably overly coupled right now.
took a closer look at the PR. probably what is needed is something more like “skip if not pyhton 3.8”. the other alternative “skip if mujoco present” seems unnecessarily constraining.
A lot of stuff has to happen before any of what I have shared becomes relevant for production, but I hope what was learned during this exercise may be useful when that time comes. I’ll park it for now.
I had this exact error, and I think I mitigated it by setting scipy version to the exact same version that comes with conda and Python 3.8. To see which one it is, you can run uv pip list --format=freeze while in an active (and working) conda environment. But not sure
I checked “chore: initial uv environment setup by jeremyshoemaker · Pull Request #321 · thousandbrainsproject/tbp.monty”, but don’t exactly follow the rationale… probably missing some larger context. In any case, in the habitat-ipc branch I commented all of these to move on, making it a bit harder to merge with main, however helpful that information may be in building a case for its removal …
The rational at the time was that while I was trying to work on adding a MuJoCo simulator and upgrading Python I ran into several issues:
issues around what versions could be installed from PyPI vs Conda
CPU architecture issues. Our Conda environment on MacOS uses x86_64 so they run through Rosetta on aarch64. MuJoCo explicitly checks for Rosetta on Apple Silicon and throws an error at import time. There was some issue the other way too, if we weren’t running Conda on x86 (probably whatever the reason was that we initially went that way on Apple Silicon), but I forget the details.
PyPI at the time had a habitat_sim package, but I don’t think it was published by the people working on it, and the version didn’t match the one we want. This might not still be true, but at the time we didn’t want to depend on untrusted packages.
and others that I forget off the top of my head.
So in order to get an environment working, I created one with uv (which is still WIP and not supported), and I marked those tests to be skipped if habitat_sim wasn’t present. You’ll notice that I did the same thing the other way around for the few MuJoCo tests that exists, though in that case I did it at the module level. I didn’t do it that way for Habitat because those tests are currently mixed in with the rest of the tests.
If you can think of improvements in how to do it, we’re open to suggestions or PRs . Ideally, we’d move away from doing full integration tests all the time, and get more coverage from unit tests that don’t depend on having a simulator available, but that’s a slow process.
Hi Jeremy, thanks for taking the time to provide this great explanation
habitat_sim will be absent not just when mujoco is present, but systematically, when the habitat sim is finally extracted to a separate process. Therefore, evaluating “am I running inside an habitat test configuration ?” won’t be possible by checking the presence of the habitat module.
One simple alternative, given that we seem to not be interested in supporting multiple simultaneous simulator types, is to acknowledge that exclusivity, meaning “mujoco is present” becomes a proxy for this.
The other question I raised was related with coverage. Until the vast majority of tests stop depending on habitat, anyone developing monty only with mujoco will keep risking bad surprises when their PRs hits the ci, but as long as that ci runs all tests for all sims, it should be fine I think.
Hope this helps. Thanks again for the great feedback !
The reason was that the dependencies we were using from Conda were not available for aarch64 because aarch64 was not a build target when those dependencies were built by the providers.
The other question I raised was related with coverage. Until the vast majority of tests stop depending on habitat, anyone developing monty only with mujoco will keep risking bad surprises when their PRs hits the ci, but as long as that ci runs all tests for all sims, it should be fine I think.
I was thinking about this problem this morning and it might make sense to try to take the current tests that rely on Habitat and isolate out the Habitat parts from the test parts so that we can try to reuse the latter for MuJoCo tests. The Habitat parts should be related to setting up the environment, and once that’s done, the tests ideally shouldn’t care about what environment they’re in.
In reality, they probably do depend on the environment they’re in since the outputs from each won’t match exactly, but that’s something we’ll need to reconcile at some point anyway.
My thinking would be to have the tests themselves in a shared base test class (or classes), and then have Habitat and MuJoCo specific test subclasses that do the setup work for their respective environments. Then, pytest can run those two sets of tests when appropriate and we can keep them in sync as much as possible during the transition.
I see the idea of having a common simulator interface nicely fitting and enabling such design, indeed.
From a future-proof point of view, the direction you describe seems very sound, but I’ve been wondering sometimes if that future is more of platform dream or a real product need, probably because I didn’t read some document about the place of simulators in the larger vision.
As I understand the situation, there would be two possible paths, one where monty becomes a bring-your-sim platform - which would fit very well the technical direction you describe - and another, where the platform would coin a direct relationship between use cases and specific simulators:
a simulator for static objects (model inference)
and another, more deterministic, for objects in motion (behavior inference).
Both would be used simultaneously, but never overlap in terms of configurations. monty would commit to specific targeted sims instead of investing in creating a sim-agnostic platform. This approach would not require so much refactoring, the current imbalance becoming less of historical accident and more of directional intent.
It could be argued that this approach would divert less resources from other areas, but it also depends on how strategic having flexibility about simulators might become for the emergence of the “killer app”, and how much the team may be able to commit on that right away or prefer leaving it open.
…