Skip to content
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

Tf event mc #235

Merged
merged 5 commits into from
Nov 16, 2018
Merged

Tf event mc #235

merged 5 commits into from
Nov 16, 2018

Conversation

YujiOshima
Copy link
Contributor

@YujiOshima YujiOshima commented Nov 5, 2018

Added a metrics collector for TF-Event file.
Related #221 #173
There is no doc now. I will add some docs.
@jlewi @gaocegege


This change is Reviewable

Signed-off-by: YujiOshima <[email protected]>
Signed-off-by: YujiOshima <[email protected]>
@jlewi
Copy link
Contributor

jlewi commented Nov 5, 2018

@YujiOshima really glad to see this coming along.

Anything we can do to help? I'm going to assk @richardsliu to be one of the lead reviewers for this.

Signed-off-by: YujiOshima <[email protected]>
Signed-off-by: YujiOshima <[email protected]>
@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Nov 7, 2018
@YujiOshima YujiOshima changed the title [WIP] Tf event mc Tf event mc Nov 7, 2018
@YujiOshima
Copy link
Contributor Author

@YujiOshima
Copy link
Contributor Author

/retest

1 similar comment
@YujiOshima
Copy link
Contributor Author

/retest

yield root
for f in files:
yield os.path.join(root, f)
def parse_summary(self, tfefile, metrics):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment with an example log and expected output?

if os.path.isdir(f):
continue
try:
print(f+" will be parse.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: parsed.

with api_pb2.beta_create_Manager_stub(channel) as client:
gsrep = client.GetStudy(api_pb2.GetStudyRequest(study_id=study_id), 10)
self.metrics = gsrep.study_config.metrics
self.Parser = TFEventFileParser()
Copy link
Contributor

Choose a reason for hiding this comment

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

self.parser

for m in ml:
mls.append(ml[m])
except:
print("Unexpected error:", sys.exc_info()[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the logging library if possible.

va.value = v.value
channel = grpc.beta.implementations.insecure_channel(opt.manager_addr, opt.manager_port)
with api_pb2.beta_create_Manager_stub(channel) as client:
print("In " + mlset.worker_id + " " + str(len(mlset.metrics_logs)) + " metrics will be reported.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the logging library if possible

Signed-off-by: YujiOshima <[email protected]>
@YujiOshima
Copy link
Contributor Author

@richardsliu Thank you for your review. I updated!

@richardsliu
Copy link
Contributor

/lgtm
/approve

@YujiOshima
Copy link
Contributor Author

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardsliu, YujiOshima

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 94b138a into kubeflow:master Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants