Closed AlanSavio25 closed 7 months ago
Hi @AlanSavio25!
Thank you for your pull request and welcome to our community.
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
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!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!
I have now merged the refactored plotting function with the original one. The plotting function takes in a flag that tells it whether the pred and batch inputs are refactored.
Clarification about the map format
Next Steps
Hi, I made some changes since my last comment. Here are a few details:
Dataloader
Transforms (wrappers.py)
Metrics
The eval results of the pre-trained model on this PR are exactly identical to those on the main branch (including GPS fusion errors). Here are the evaluation results on a recently downloaded MGL dataset:
{'xy_expectation_error': tensor(9.6098), 'xy_max_error': tensor(10.4407), 'xy_recall_2m': tensor(0.3564), 'xy_recall_5m': tensor(0.5960), 'yaw_max_error': tensor(22.3918), 'yaw_recall_2°': tensor(0.4066), 'yaw_recall_5°': tensor(0.7038), 'directional_error': tensor([6.5439, 6.5661]), 'xy_gps_error': tensor(4.5306), 'xy_fused_error': tensor(7.1373), 'yaw_fused_error': tensor(18.7499)}
[2024-04-04 20:43:37 maploc INFO] Recall xy_max_error: [14.02, 47.57, 59.6] at (1, 3, 5) m/°
[2024-04-04 20:43:37 maploc INFO] Recall xy_gps_error: [24.8, 56.46, 66.14] at (1, 3, 5) m/°
[2024-04-04 20:43:37 maploc INFO] Recall yaw_max_error: [21.82, 55.52, 70.38] at (1, 3, 5) m/°
They are also similar to the results on README.md, which are:
Recall xy_max_error: [14.37, 48.69, 61.7] at (1, 3, 5) m/°
Recall yaw_max_error: [20.95, 54.96, 70.17] at (1, 3, 5) m/°
Minor Note: When evaluating with has_gps=True
, the GPS fusion metrics are exactly identical between main
and refactoring
when gaussian
is set to True
in fuse_gps()
, but there is a 0.12m difference on the xy_fusion_error
metric when gaussian=False
. Not sure why exactly
I trained main
and refactoring
for 24h each (~328k steps, batch size=1) on an RTX 2080, and the loss curves are similar. The main difference between the two is that in refactoring
, the flip-augmented training images are correctly rectified.
The eval results after training are shown below:
main
All results: {'xy_expectation_error': tensor(18.0240), 'xy_max_error': tensor(21.0109), 'xy_recall_2m': tensor(0.1136), 'xy_recall_5m': tensor(0.2700), 'yaw_max_error': tensor(55.6853), 'yaw_recall_2°': tensor(0.1816), 'yaw_recall_5°': tensor(0.3569), 'directional_error': tensor([12.5002, 14.1251])}
[2024-04-05 13:51:24 maploc INFO] Recall xy_max_error: [4.03, 18.16, 27.0] at (1, 3, 5) m/°
[2024-04-05 13:51:24 maploc INFO] Recall yaw_max_error: [9.58, 25.69, 35.69] at (1, 3, 5) m/°
refactoring
All results: {'xy_expectation_error': tensor(17.3318), 'xy_max_error': tensor(20.2708), 'xy_recall_2m': tensor(0.1151), 'xy_recall_5m': tensor(0.2889), 'yaw_max_error': tensor(47.4610), 'yaw_recall_2°': tensor(0.1805), 'yaw_recall_5°': tensor(0.3972), 'directional_error': tensor([12.3601, 13.4812])}
[2024-04-05 13:29:57 maploc INFO] Recall xy_max_error: [3.4, 18.94, 28.89] at (1, 3, 5) m/°
[2024-04-05 13:29:57 maploc INFO] Recall yaw_max_error: [[9.84, 26.11, 39.72]] at (1, 3, 5) m/°
Hi @sarlinpe,
As a first step towards refactoring OrienterNet, I made some changes to the model outputs and adapted the visualization code to work with the new indexing:
I couldn't get the circle inset to work with the new indexing, so I've temporarily disabled it in the refactored plotting function.
Would love to hear any suggestions you might have on any of the changes!