Skip to content

Optimizations#24

Open
Gushroom wants to merge 13 commits intomasterfrom
optimizations
Open

Optimizations#24
Gushroom wants to merge 13 commits intomasterfrom
optimizations

Conversation

@Gushroom
Copy link
Copy Markdown
Collaborator

  1. Move entire embedding_store to pinned CPU memory, saves 7GB CUDA memory when using full dim features, only load slices as needed. Increased batch sizes everywhere because bulk is released.
  2. Have semantic term and dense depth disparity term share a projection and cosine similarity cache, so they dont recompute.
  3. Use pytorch's gridsample to replace handwritten bilinear_sample functions, use autograd for the require grad version.
  4. Early stopping in inner BA loop.
  5. GPU-based solver, replaces scipy cpu solver when system dim >= 150, avoids 2 memory loads per solve, and is faster as system dim grows.
  6. Reduced update() iters from 3 to 1, sounds dangerous somehow just works, maybe frontend doesnt need so much BA anyways.
    result of master(fixed basics so it would run):
    room0,0.03825371247444848
    room1,0.01005237004357161
    room2,0.057475465760270746
    office0,0.03942505742579552
    office1,0.11074517577864879
    office2,0.08503124845291919
    office3,0.03086148128370234
    office4,0.03128040904201112
    rgbd_dataset_freiburg3_walking_xyz,0.01638278260556373
    rgbd_dataset_freiburg3_walking_rpy,0.032532340569786905
    rgbd_dataset_freiburg3_walking_halfsphere,0.02115683070638981
    rgbd_dataset_freiburg3_walking_static,0.005543244622892279
    rgbd_dataset_freiburg3_sitting_xyz,0.01051249305306071
    rgbd_dataset_freiburg3_sitting_rpy,0.025846121224890568
    rgbd_dataset_freiburg3_sitting_halfsphere,0.016400059590165477
    rgbd_dataset_freiburg3_sitting_static,0.005484478207241217
    result as of PR:
    room0,0.03749123880664903
    room1,0.008577582840133742
    room2,0.056811805273262364
    office0,0.0391795609748814
    office1,0.08516443380078807
    office2,0.08865144726319692
    office3,0.02947708054431146
    office4,0.032610775559089075
    rgbd_dataset_freiburg3_walking_xyz,0.016106582739058756
    rgbd_dataset_freiburg3_walking_rpy,0.030542095363680407
    rgbd_dataset_freiburg3_walking_halfsphere,0.020357887640457668
    rgbd_dataset_freiburg3_walking_static,0.005579860666927469
    rgbd_dataset_freiburg3_sitting_xyz,0.010530894028695118
    rgbd_dataset_freiburg3_sitting_rpy,0.02591678430689701
    rgbd_dataset_freiburg3_sitting_halfsphere,0.01661740119671936
    rgbd_dataset_freiburg3_sitting_static,0.0055112765165804874

@Gushroom Gushroom requested a review from JaafarMahmoud1 April 14, 2026 03:10
init:
camera_type: "pinhole"
intrinsics: "file"
intrinsics: "geocalib"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make the default settings to read from file, but fall back to geocalib with Warning if File didn't exist.
Also Add in Readme, example of the intrinsics @zaid-nasser

Comment thread configs/pipeline/tum.yaml
init:
camera_type: "pinhole"
intrinsics: "file"
intrinsics: "geocalib"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, default is file, and fall back if file not exited. @zaid-nasser

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's merge this into master and add this as a separate feature?

Comment thread configs/pipeline/tum.yaml
radseg_lang_align: False
pca_dim: 256
pca_dim: null
pca_state_path: "/home/user/km-vipe/vipe_results/vipe/pca_basis.pt"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zaid-nasser Also let's fix the logic here, and remove this path, and make it relative in all settings.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a placeholder, it will be overwritten by whatever that's passed in the running script, but there is a subtle issue here, this field cannot be none.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants