Skip to content

Ogbench#6

Open
helenlu66 wants to merge 2 commits into
mainfrom
ogbench
Open

Ogbench#6
helenlu66 wants to merge 2 commits into
mainfrom
ogbench

Conversation

@helenlu66
Copy link
Copy Markdown
Member

just adding ogbench as a submodule

Copy link
Copy Markdown
Member

@pranavguru pranavguru left a comment

Choose a reason for hiding this comment

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

We should handle the submodule better - I think we faced similar issues in v1.0 where the pi0 submodule was not updated and synced. We should not let that happen with v2.0.

Comment thread .vscode/launch.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does this file need to be pushed to the repo? Seems like a developer config file

Comment thread ogbench
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Submodule is unclonable. Two independent issues:

  1. .gitmodules declares url = ./ogbench. Git resolves relative submodule URLs against the parent repo's origin, producing https://github.com/ManifoldRG/MultiNet-v2.0.git/ogbench, which doesn't exist. Repro:
$ git submodule update --init --recursive
fatal: repository 'https://github.com/ManifoldRG/MultiNet-v2.0.git/ogbench/' not found
  1. Fix: replace with an absolute URL.

  2. The pinned SHA d7509418e7a2243ebea3c5fc081ed57557e89719 is not reachable on canonical seohongpark/ogbench:

$ git ls-remote https://github.com/seohongpark/ogbench.git | grep d7509418
 (no output)
  1. So even after fixing the URL, the submodule won't resolve unless we point at the fork that actually contains those commits

Before merging, we need to:

  • Confirm where SHA d7509418e7… actually lives. If it's on a personal fork, push it to a ManifoldRG/ogbench fork instead so the pin doesn't depend on a personal account.
  • Update .gitmodules to the absolute URL of that fork.
  • Open an upstream PR (or a long-running fork branch) for the OGBench-internal D2 maze fixes, so we have a path to eventually re-sync with seohongpark/ogbench and not maintain a permanent fork.

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.

2 participants