-
Notifications
You must be signed in to change notification settings - Fork 533
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
Add split_eval_set command #1609
Conversation
Signed-off-by: Jimmy Xu <[email protected]>
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.
Looks reasonable, would want to also see a test run on MCT using this script
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
Please also add a PR description (describing the functionality added, not any downstream API needs). |
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
|
||
log = logging.getLogger(__name__) | ||
|
||
LOCAL_PATH = 'tmp-t' |
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 don't think you want to hardcode this. Should this not just be a fallback to assume local path?
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.
im not too familiar, is it acceptable to have
if remote object store:
remote object store:
else:
local path
i.e. have it be the defualt?
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.
seems ok? just note that is what it defaults to
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
Signed-off-by: Jimmy Xu <[email protected]>
closing this PR as we are re-prioritizing the long term workflows solution over pushing out a sweep/eval PuPr asap, which this PR was meant to do |
No description provided.