-
Notifications
You must be signed in to change notification settings - Fork 113
demo docker #539
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
demo docker #539
Conversation
boomanaiden154
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I'm assuming the plan is to write a README at some point on how to run everything locally?
Do we have performance numbers yet for how well ES training works on this dataset and what sort of performance people should expect?
| cmake -B /work/llvm-corpus \ | ||
| -GNinja \ | ||
| -DCMAKE_EXPORT_COMPILE_COMMANDS=On \ | ||
| -DCMAKE_BUILD_TYPE=Release \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be MinSizeRel to ensure we're compiling the modules with -Os or -Oz (I forget which one it is)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it should be "MinSizeRel" if you are training for inlining
|
What's the reason for breaking each step into its own script (which is basically a one-liner)? |
Would let someone skip / redo steps. If it's all in a file, there's a tendency that the file would evolve into a monolithic unit, with variables and reusable things and whatnot, and in this case, I want to avoid this. |
eventually.
No, and we don't yet have its own gin files either. |
This is the initial step. It currently still uses the checked in gin files and it doesn't yet fix a git hash for this repo (the latter because we need at least this commit to land first). Also, pre-training (warmstart) isn't implemented yet.
see also this RFC
Issue #540