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

Tuning halving's zipkin tracing text is set to stacking.training #359

Open
jingren1021 opened this issue Nov 21, 2022 · 4 comments
Open
Labels
bug Something isn't working

Comments

@jingren1021
Copy link
Contributor

I am not sure if this is an oversight or intended.

Zipkin tracing text is set to stacking.training for tuning halving benchmark.
image

Also, most proto files in tuning halving uses stacking as name instead of tuning.
For example , tuning_pb2.py in tuning-halving/proto/ has descriptor as below.

DESCRIPTOR = _descriptor.FileDescriptor(
  name='tuning.proto',
  package='stacking',
  syntax='proto3',
  serialized_options=b'\n\022com.vhive.stackingB\010stackingP\001Z\035tests/stacking-training/proto',
  create_key=_descriptor._internal_create_key,
  serialized_pb=b'\n\x0ctuning.proto\x12\x08stacking\"n\n\x0cTrainRequest\x12\x0f\n\x07\x64\x61taset\x18\x01 \x01(\x0c\x12\x13\n\x0b\x64\x61taset_key\x18\x02 \x01(\t\x12\x14\n\x0cmodel_config\x18\x03 \x01(\x0c\x12\r\n\x05\x63ount\x18\x04 \x01(\x03\x12\x13\n\x0bsample_rate\x18\x05 \x01(\x02\"_\n\nTrainReply\x12\r\n\x05model\x18\x01 \x01(\x0c\x12\x11\n\tmodel_key\x18\x02 \x01(\t\x12\x10\n\x08pred_key\x18\x03 \x01(\t\x12\r\n\x05score\x18\x04 \x01(\x02\x12\x0e\n\x06params\x18\x05 \x01(\x0c\x32\x42\n\x07Trainer\x12\x37\n\x05Train\x12\x16.stacking.TrainRequest\x1a\x14.stacking.TrainReply\"\x00\x42?\n\x12\x63om.vhive.stackingB\x08stackingP\x01Z\x1dtests/stacking-training/protob\x06proto3'
)
@ustiugov ustiugov added the bug Something isn't working label Nov 21, 2022
@ustiugov
Copy link
Member

definitely a bug. thanks for the reporting!

@ustiugov
Copy link
Member

@jingren1021 could you add these minor changes to your PR for the tuning workload?

@jingren1021
Copy link
Contributor Author

jingren1021 commented Nov 24, 2022

I have fixed the benchmarks/tuning-halving/proto/tuning.proto in this commit.

However, I noticed that the docker images still use tuning_pb2.py and tuning_pb2_grpc.py generated from old protocol buffer compiler. Same with the current commit of repository. Is there a pipeline to update the proto directory for both repository and docker images or it has to be done manually?

@ustiugov
Copy link
Member

@jingren1021 I am afraid it is manual now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants