-
Notifications
You must be signed in to change notification settings - Fork 315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improve model properties table in docs #769
base: dev
Are you sure you want to change the base?
improve model properties table in docs #769
Conversation
61c245a
to
c086f8c
Compare
…com/mivanit/TransformerLens into improve-model-properties-table-docs
adding things one at a time to see what causes things to break
note: when trying to run the tests with no changes from dev, docs build fails due to missing access to
|
we still generate a plain markdown table code is from the old PR: https://github.com/mivanit/TransformerLens/blob/add-better-model-properties-table/docs/make_docs.py which is in turn a modified version of https://github.com/mivanit/transformerlens-model-table
…fault device meta
currently it seems like everything should work (still waiting on the action to finish), except:
Error Details
For the time being, I have set get_model_table(
model_table_path=GENERATED_DIR / "model_table.jsonl",
force_reload=True,
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
allow_except=True, # TEMPORARY: until HF_TOKEN in secrets allows access to models:
# mistral-7b mistral-7b-instruct mistral-nemo-base-2407 mixtral mixtral-instruct
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
) everything between the
# Model Properties Table
also see the [interactive model table](../_static/model_properties_table_interactive.html) I am not sure what the best practice is here for where to put the new table and how to link to it. I do think it's worth keeping the static markdown table because it loads much faster and doesn't rely on JS to work. |
That is very odd. The token in there should be mine, and I should have access to all of those models. Can you remove your change that allows it to pass those models? I will take a look and investigate further this coming week. |
dcfb6e8 passes all tests and doc building (with mistral models being skipped) and |
…erLens into branch on mivanit fork ran `poetry update` due to `poetry.lock` conflict
managed to fix this by resetting lockfile to version from main and running |
I'm still getting failures on these 5 models, that appears to be unchanged. I'm also getting "soft" failures on a variety of models, where the function is also unable to use the HF token. Could it be the case that as someone who isn't an authorized contributor to the repo, the repo secrets are not provided when I initialize a github action? This would make sense since otherwise any random person could submit a PR that prints the secrets and thus steal them. |
Ready for review, requires
HF_TOKEN
to allow access to certain gated mistral modelsDescription
This PR ports features from my transformerlens-model-table repo to TransformerLens, implementing most of the features requested in #97. I still need some feedback on this, and presumably building docs will fail for one reason or another once I make the PR.
Features:
The static table has a few more fields added to it, but the primary focus is the interactive table. This provides:
meta
, doesn't require actually loading models)Adds dependencies
under group
docs
:tiktoken
for dealing with certain tokenizersmuutils
for pretty-printed data on tensor shapesType of change
Screenshots
Before:
Original model properties table
After (static):
You can see what the generated data looks like here
After (interactive):
See demo
Checklist:
(currently draft PR, testing incomplete)
Notes:
docs
in the branch name, and I was also having issues with tests unrelated to my change failing.