Skip to content

Factor executor code+history from marin-community/marin#3

Closed
ryan-williams wants to merge 307 commits intomainfrom
rw/ps2.3
Closed

Factor executor code+history from marin-community/marin#3
ryan-williams wants to merge 307 commits intomainfrom
rw/ps2.3

Conversation

@ryan-williams
Copy link
Copy Markdown
Member

@ryan-williams ryan-williams commented Aug 29, 2025

Update: superseded by #4


  • ✅ Use filter-marin-executor.sh to extract executor code
    • Run git-filter-repo on a Marin clone:
      • Create filtered history (just executor and related files)
      • In PR-merge-commit subjects, rewrite #XXX to marin-community/marin#XXX
    • Create empty initial commit f27eaad
    • Rebase filtered history on it
  • ✅ Add commits:
    • 3b2ccdd:
      • Add project files whose history wasn't worth replaying (e.g. README.md)
      • Rename src/{marin,thalas}/, update imports
    • 812031d: skip a macOS-failing test
  • ✅ Push empty initial commit (f27eaad) to main
  • ⏳ This PR: rewritten history + subsequent changes → main

See gist for scripts and more info.

abhinavg4 and others added 30 commits October 6, 2024 23:14
# Conflicts:
#	.github/workflows/quickstart-test.yaml
…imPajama build (marin-community/marin#447)

* wip

* reduce the number of "shard groups" to something manageable, makes SlimPajama build

* lint
Helw150 and others added 10 commits July 1, 2025 12:07
…arin#1364)

* Initial Gemstone Scraping

* Not Quite Working, But getting there

* Only run evals of the cooled down WSD models (Too Many otherwise lol)

* Final Version

* International Corpus of English is Private

* This just can't be dry run due to config loading
…running again (marin-community/marin#1442)

* Refactor executor to run steps directly

* working on apply autoscaler patch to ray 2.45

* fix deps for uv

* tests?

* wip

* ok it runs!

* fix build

* stupid ray

* oops

* sigh

* missed the lock

* grr

* grr2

* more uv run

* no space?!?

* tpu tests at least?

* wtf du

* more du

* am faf

* again

* this?

* grr

* pr comments
…1479)

* actually schedule statusactor on the headnode

* Update marin/utilities/ray_utils.py

Co-authored-by: William Held <Wbh230@nyu.edu>

---------

Co-authored-by: William Held <Wbh230@nyu.edu>
…ty/marin#1453)

* Use network_endpoints for expected TPU workers

* Refactor executor to run steps directly

* working on apply autoscaler patch to ray 2.45

* fix deps for uv

* tests?

* wip

* ok it runs!

* fix build

* Refine TPU monitor

* stupid ray

* oops

* sigh

* missed the lock

* grr

* grr2

* more uv run

* no space?!?

* tpu tests at least?

* wtf du

* more du

* am faf

* again

* this?

* grr

* background thread for monitor

* simple main to try it out

* auto-create

* update docker and some deps

* Add entrypoint resource options and TPU support to ray run

* east 5, update clusterwqa

* src layout?

* src layout

* tweaks to east5a cluster, enw image

* make dockerfile work with uv and src layout

* uv is causing problems

* make uv want to install cpu torch (i think?)

* increase max concurrent connections for setting up TPU nodes to 64

* wip

* update cluster configs with latest image, hopefully faster tpu setup

* wip

* uv lock

* wip

* wip

* remove tpu monitor for now

* lock

* no locked

* sigh

* i hate python

* i hate python

* lock

* mkdocs

* sigh

* hrm, seems not ideal
This was referenced Aug 29, 2025
@ryan-williams ryan-williams requested review from Copilot and eric-czech and removed request for Copilot August 29, 2025 03:46
Copy link
Copy Markdown
Member

@dlwh dlwh left a comment

Choose a reason for hiding this comment

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

lgtm. we hsould fix licenses but I'm having codex do a thing for Marin and we can port here

@@ -0,0 +1,201 @@
Apache License
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.

given we're forking a chunk of Marin we should probably be sure to put Stanford on the copyright of Marin and make sure it's credited here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, did my best to mimic/adapt Marin's copyright and AUTHORS.md in #4 (specifically 112e5f0). lmk (here or there) if that seems OK?

Copy link
Copy Markdown
Collaborator

@eric-czech eric-czech left a comment

Choose a reason for hiding this comment

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

LGTM other than figuring out what to do with #3 (comment) and #3 (comment)

Copy link
Copy Markdown
Member Author

@ryan-williams ryan-williams left a comment

Choose a reason for hiding this comment

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

I've improved the history-rewriting in #4, which I propose we shift focus to. I've linked this PR's 3 open threads over there, happy to continue discussing them here or there.

I responded on the JSON ser/de thread here/below; I partly made a new PR because force-pushing here would orphan that thread, which has good discussion.

Copy link
Copy Markdown
Member

@alxmrs alxmrs left a comment

Choose a reason for hiding this comment

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

I started reviewing this yesterday and then got side tracked. Here's what I've noticed so far and I'll continue the review now.

```bash
git clone https://github.com/Open-Athena/thalas.git
cd thalas
uv venv --python 3.11
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.

TIL. I always use uv sync + the .python-version file -- this is nice to test multiple versions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting, yea I'm fumbling through how best to manage multiple venvs in a project, in uv world. I've used Pyenv for many years, which uses .python-version in a way that conflicts with uv. Open to suggestions.

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.

I'm surprised to hear that .python-version conflicts with uv! I use that feature all the time -- it seems to work for me. Maybe I misunderstand how you're using it? IIRC, when you uv init a new project it generates .python-version for you.

uv venv --python 3.11
source .venv/bin/activate
uv pip install -e .[dev]
pre-commit install
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.

optional: you could use uvx to install the pre-commit hooks (without specifically having to install pre-commit locally):

Suggested change
pre-commit install
uvx pre-commit install

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting, afaict it's better to have the version pinned/managed in pyproject.toml, otherwise we're either not pinning, or have to specify version during each invocation?

Copy link
Copy Markdown
Member

@alxmrs alxmrs left a comment

Choose a reason for hiding this comment

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

LGTM. It's nice we can see all these sources in one PR.

Copy link
Copy Markdown
Member Author

@ryan-williams ryan-williams left a comment

Choose a reason for hiding this comment

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

I believe I've addressed all comments over in #4, will close this now, but feel free to respond to existing threads here if it's easier and further discussion is warranted.

```bash
git clone https://github.com/Open-Athena/thalas.git
cd thalas
uv venv --python 3.11
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting, yea I'm fumbling through how best to manage multiple venvs in a project, in uv world. I've used Pyenv for many years, which uses .python-version in a way that conflicts with uv. Open to suggestions.

uv venv --python 3.11
source .venv/bin/activate
uv pip install -e .[dev]
pre-commit install
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting, afaict it's better to have the version pinned/managed in pyproject.toml, otherwise we're either not pinning, or have to specify version during each invocation?

@@ -0,0 +1,201 @@
Apache License
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point, did my best to mimic/adapt Marin's copyright and AUTHORS.md in #4 (specifically 112e5f0). lmk (here or there) if that seems OK?

Copy link
Copy Markdown
Member Author

@ryan-williams ryan-williams left a comment

Choose a reason for hiding this comment

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

Addressing a couple more comments

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.