Testing a bare minimum of HITL functionality #1787

henrysamer · 2024-02-02T23:52:10Z

Motivation and Context

Add ability to test HITL apps in CI
Ensure habitat-hitl can be installed alongside habitat-lab and habitat-baselines
Improve testability by using Compose API instead of @hydra.main

How Has This Been Tested

tested locally

Types of changes

[Experiment]
Created minimal HITL app in test_main_hitl.py
Runs app for 1 step using headless mode
Uses Compose API to load config
Added habitat-hitl installation to .circleci/config.yml

Checklist

[x ] My code follows the code style of this project.
[ x] I have updated the documentation if required.
[ x] I have read the CONTRIBUTING document.
[ x] I have completed my CLA (see CONTRIBUTING)
[ x] I have added tests to cover my changes if required.

eundersander

I left some comments. I expect part of the problem with this failing test is that you aren't downloading hab3-episodes (something like python -m habitat_sim.utils.datasets_download --uids hab3-episodes). Let's make sure this test is running through pytest and passing before we merge. Let me know when to take another look!

eundersander · 2024-02-05T22:01:19Z

test/hitl/test_hitl_main.py

+    main(cfg)
+
+
+if __name__ == "__main__":


This block shouldn't be needed. Pytest should run test_hitl_main automatically.

eundersander · 2024-02-05T22:01:41Z

habitat-hitl/habitat_hitl/core/hitl_main.py

@@ -43,7 +43,7 @@ def _parse_debug_third_person(hitl_config, viewport_multiplier=(1, 1)):
    return do_show, width, height


-def hitl_main(app_config, create_app_state_lambda=None):
+def hitl_main(app_config, create_app_state_lambda=None, test_flag=False):


I don't see that you're using this. I guess it could be removed.

eundersander · 2024-02-05T22:03:28Z

.circleci/config.yml

+              . activate habitat; cd habitat-lab
+              export PYTHONPATH=.:$PYTHONPATH
+              export MULTI_PROC_OFFSET=0 && export MAGNUM_LOG=quiet && export HABITAT_SIM_LOG=quiet
+              python test/hitl/test_hitl_main.py -s


We shouldn't need to run this file explicitly. We should use pytest like the other tests in this file.

eundersander · 2024-02-07T21:51:51Z

.circleci/config.yml

+              export PATH=$HOME/miniconda/bin:/usr/local/cuda/bin:$PATH
+              . activate habitat; cd habitat-lab
+              ln -s ../habitat-sim/data data
+              python -m habitat_sim.utils.datasets_download --uids hab3-episodes habitat_humanoids hab_spot_arm ycb hssd-hab --data-path data/ --no-replace --no-prune


We can't download hssd-hab in a CI test--it is quite large (~20GB). Talk to @aclegg3 about a CI-friendly version of this dataset.

https://huggingface.co/datasets/ai-habitat/hab3_bench_assets

FYI @henrysamer we still need to change this PR to not download hssd-hab. I realize the test is passing, but we shouldn't have CI doing such a large download.

Might need to understand why dataset episodes aren't being found here

…throw_vr, basic_viewer, and rearrange

…n for config.yml in .circleci

…n for config.yml in .circleci 2

…n for config.yml in .circleci

eundersander

Some more comments. Good progress!

eundersander · 2024-02-07T23:18:01Z

habitat-hitl/habitat_hitl/config/test_cfg.yaml

@@ -0,0 +1,20 @@
+# test_cfg.yaml


As this is a config for testing only, it should probably live with the tests, not in the library itself. test/config/habitat_hitl/test_cfg.yaml would be consistent with the other test configs.

eundersander · 2024-02-07T23:18:17Z

test/hitl/test_hitl_main.py

+    ):
+        cfg = compose(config_name="test_cfg")
+    main(cfg)
+    print("done!")


No need to print this.

eundersander · 2024-02-07T23:20:08Z

my_session.0.json.gz

No need to add these binary output files to source control.

eundersander · 2024-02-07T23:30:17Z

.circleci/config.yml

+
+      - run:
+          name: Run HITL functionality
+          no_output_timeout: 60m


It looks like your recent successful run took about 4 minutes. I'm actually shocked it downloaded HSSD so quickly! Anyway let's reduce this timeout to something like 10 or 15 minutes.

eundersander · 2024-02-07T23:31:09Z

my_session.0.json.gz

No need to add these binary output files to source control.

eundersander · 2024-02-07T23:31:43Z

.circleci/config.yml

+              python -m habitat_sim.utils.datasets_download --uids hab3-episodes habitat_humanoids hab_spot_arm ycb hssd-hab --data-path data/ --no-replace --no-prune
+              export PYTHONPATH=.:$PYTHONPATH
+              export MULTI_PROC_OFFSET=0 && export MAGNUM_LOG=quiet && export HABITAT_SIM_LOG=quiet
+              export TEST_BASELINE_SMALL=1


I don't think this env var TEST_BASELINE_SMALL is needed for your test.

…n for config.yml in .circleci

…p_recent_server_keyframe_id to help the server measure client latency (#1796)

* Add type annotations to app_service. * Propagate AppService type annotations to the apps. * Move displaced comment. * Force keyword argument usage in AppService constructior.

…sing pipeline. (#1789)

* correction * correction * correction * correction * correction * correction * correction

…mo (#1810) * nicer instructions for installing Magnum on Mac * add MVP rearrange_v2 HITL app * add GuiPlacementHelper * add ClientHelper * changes to GuiPickHelper. * add config habitat_hitl.debug_line_width * changes to how connection_record/connection_params are handled * add config habitat_hitl.networking.wait_for_app_ready_signal * support for tracking client latency * add color and billboard-versus-up-facing options for highlight messages * minor bugfix to unity_dataset_processing.py * add hydra-core to requirements.txt

* Add camera sync support to HITL. * Renaming and refactoring.

* Pass skinning keyframe data to the client. * Add configuration to activate or deactivate skinning. * Formatting fix. * Add checks for rig removal from keyframes. * Refactoring. Remove conditional consolidation of rigs.

facebook-github-bot · 2024-02-26T18:06:41Z

Hi @henrysamer!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

henrysamer requested a review from eundersander February 2, 2024 23:52

henrysamer self-assigned this Feb 2, 2024

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 2, 2024

henrysamer added the do not merge Not ready to merge. This label should block merging. label Feb 3, 2024

henrysamer changed the title ~~Testing a bare minimum of HITL functionality including minimal, pick_throw_vr, basic_viewer, and rearrange~~ Testing a bare minimum of HITL functionality Feb 5, 2024

henrysamer removed the do not merge Not ready to merge. This label should block merging. label Feb 5, 2024

eundersander reviewed Feb 5, 2024

View reviewed changes

henrysamer marked this pull request as ready for review February 7, 2024 12:41

eundersander reviewed Feb 7, 2024

View reviewed changes

henrysamer added 20 commits February 7, 2024 18:25

testing a bare minimum of HITL functionality including minimal, pick_…

5b55410

…throw_vr, basic_viewer, and rearrange

testing a bare minimum of HITL functionality + added hitl installatio…

e600374

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

3745927

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

7333cc3

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

222475d

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

9a91a51

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

64aa513

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

16c8426

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

44e8468

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

e5126b9

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

741681f

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

888625e

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

b22c8bf

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

f5b5c62

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

f207599

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

69d332d

…n for config.yml in .circleci 2

testing a bare minimum of HITL functionality + added hitl installatio…

d661244

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

30aa077

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

a83799e

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

b9e397c

…n for config.yml in .circleci

henrysamer force-pushed the hitl_test1 branch from 3185a37 to b9e397c Compare February 7, 2024 23:27

eundersander reviewed Feb 7, 2024

View reviewed changes

henrysamer and others added 17 commits February 9, 2024 10:07

testing a bare minimum of HITL functionality + added hitl installatio…

31d6924

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

7d9115e

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

951ff27

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

b4766b1

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

dd17ff5

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

8f4515d

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

c7f8b27

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

6fece47

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

168a295

…n for config.yml in .circleci

testing a bare minimum of HITL functionality + added hitl installatio…

367072e

…n for config.yml in .circleci

add ClientMessageManager.set_server_keyframe_id and RemoteGuiInput.po…

4292d6b

…p_recent_server_keyframe_id to help the server measure client latency (#1796)

HITL: Add typing annotations to AppService (#1802)

a33bf0e

* Add type annotations to app_service. * Propagate AppService type annotations to the apps. * Move displaced comment. * Force keyword argument usage in AppService constructior.

Add use-multiprocessing and verbose flags to the Unity dataset proces…

8c1f48c

…sing pipeline. (#1789)

Correct object initialization (#1806)

2a31734

* correction * correction * correction * correction * correction * correction * correction

HITL: Add camera sync support. (#1811)

8562e63

* Add camera sync support to HITL. * Renaming and refactoring.

HITL: Make skinning support optional. (#1791)

fdcb131

* Pass skinning keyframe data to the client. * Add configuration to activate or deactivate skinning. * Formatting fix. * Add checks for rig removal from keyframes. * Refactoring. Remove conditional consolidation of rigs.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testing a bare minimum of HITL functionality #1787

Testing a bare minimum of HITL functionality #1787

henrysamer commented Feb 2, 2024 •

edited

eundersander left a comment

eundersander Feb 5, 2024

eundersander Feb 5, 2024

eundersander Feb 5, 2024

eundersander Feb 7, 2024

aclegg3 Feb 8, 2024

eundersander Feb 12, 2024

henrysamer Feb 15, 2024

eundersander left a comment

eundersander Feb 7, 2024

eundersander Feb 7, 2024

eundersander Feb 7, 2024

eundersander Feb 7, 2024

eundersander Feb 7, 2024

eundersander Feb 7, 2024

facebook-github-bot commented Feb 26, 2024

Testing a bare minimum of HITL functionality #1787

Are you sure you want to change the base?

Testing a bare minimum of HITL functionality #1787

Conversation

henrysamer commented Feb 2, 2024 • edited

Motivation and Context

How Has This Been Tested

Types of changes

Checklist

eundersander left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eundersander left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Feb 26, 2024

Process

henrysamer commented Feb 2, 2024 •

edited