kubeflow / katib

Automated Machine Learning on Kubernetes
https://www.kubeflow.org/docs/components/katib
Apache License 2.0
1.5k stars 441 forks source link

[file-metricscollector]: metrics should be formatted as string #2162

Closed axel7083 closed 1 year ago

axel7083 commented 1 year ago

The following line, will try to cast a string any value provided.

https://github.com/kubeflow/katib/blob/37b237f56091a36db35cf4a4148847d1da572278/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector.go#L139

This will make fail any other value (float, double etc.). If this is the expected behavior, it should probably be documented. The example available in the katib documentation shows as example:

{"epoch": 0, "foo": “bar", “fizz": “buzz", "timestamp": 1638422847.28721…}
{"epoch": 1, "foo": “bar", “fizz": “buzz", "timestamp": 1638422847.287801…}
{"epoch": 2, "foo": “bar", “fizz": “buzz", "timestamp": "2021-12-02T14:27:50.000035161+09:00"…}
{"epoch": 3, "foo": “bar", “fizz": “buzz", "timestamp": "2021-12-02T14:27:50.000037459+09:00"…}

Does not really shows that the value should be between ".

tenzen-y commented 1 year ago

@axel7083 Thanks for creating this issue. I'm a creator of the feature. Casting to string is intended since I assumed to output the log by hypertune, which outputs string value.

https://github.com/GoogleCloudPlatform/cloudml-hypertune/blob/8e3530e1c4926ac64cf28330d1104b838e07a468/hypertune/hypertune.py#L64-L74

Maybe, it is hard to support non-string values since we can not know which type the value is using in advance.

So I agree with mentioning the limitation in the doc.

cc: @kubeflow/wg-automl-leads

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 year ago

This issue has been automatically closed because it has not had recent activity. Please comment "/reopen" to reopen it.