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

Support experiments with generate_name feature #2243

Closed
fensterwetter opened this issue Nov 14, 2023 · 8 comments
Closed

Support experiments with generate_name feature #2243

fensterwetter opened this issue Nov 14, 2023 · 8 comments
Assignees

Comments

@fensterwetter
Copy link

/kind feature

Describe the solution you'd like
The Python katib client (again) supports experiments with the generate_name feature.

Anything else you would like to add:
After upgrading from kubeflow-katib 0.14.0 to 0.16.0, I noticed that the create_experiment function in katib_client.py now implicitly makes the assumption that the experiment object contains an attribute called .metadata.name. However, generally name can be missing in favour of generate_name which is then treated as a prefix which gets an additional suffix from the server. In version 0.14.0, the server return was used to access the resulting final name. At least from kubeflow-katib version 0.15.0 onwards, the server return is no longer captured and the original experiment object is used to access the name attribute.

I was wondering if the change was a conscious decision or rather a side effect that could be reverted. I am also happy to collaborate in case this is something that makes sense for katib to support.

Thanks
Thomas


Love this feature? Give it a 👍 We prioritize the features with the most 👍

@tenzen-y
Copy link
Member

That makes sense. We should get the generated name if the name is empty here:

except multiprocessing.TimeoutError:
raise TimeoutError(
f"Timeout to create Katib Experiment: {namespace}/{experiment.metadata.name}"
)
except Exception:
raise RuntimeError(
f"Failed to create Katib Experiment: {namespace}/{experiment.metadata.name}"
)
# TODO (andreyvelich): Use proper logger.
print(f"Experiment {namespace}/{experiment.metadata.name} has been created")

cc: @andreyvelich

@bharathk005
Copy link
Contributor

@tenzen-y I can work on this if this feature is still needed.

@tenzen-y
Copy link
Member

@tenzen-y I can work on this if this feature is still needed.

Yes, I believe that this is still needed. Feel free to open the PR.

@bharathk005
Copy link
Contributor

@tenzen-y
Do we need to handle this scenario in the backend as well?

err = k.katibClient.CreateRuntimeObject(&job)

@tenzen-y
Copy link
Member

tenzen-y commented Feb 27, 2024

@tenzen-y Do we need to handle this scenario in the backend as well?

err = k.katibClient.CreateRuntimeObject(&job)

@bharathk005 That backend is only for Katib UI. So, we can improve only SDK.

@tenzen-y
Copy link
Member

tenzen-y commented Mar 4, 2024

@bharathk005 Could you say /assign if you are already working on this?

@bharathk005
Copy link
Contributor

/assign

@andreyvelich
Copy link
Member

This should be fixed with: #2272

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants