Skip to content

Conversation

@ParagEkbote
Copy link
Contributor

@ParagEkbote ParagEkbote commented Mar 17, 2025

Fixes #269

I've setup an example with the scikit-learn breast cancer dataset and a Random Forest Classifier model. How do I set up the comet API key for CI?

cc: @not522

@ParagEkbote ParagEkbote marked this pull request as draft March 17, 2025 18:22
@ParagEkbote
Copy link
Contributor Author

ParagEkbote commented Mar 21, 2025

Output of experiment for optimizing the f1 score:

f1_score

@ParagEkbote ParagEkbote marked this pull request as ready for review March 22, 2025 05:47
@HideakiImamura
Copy link
Member

@ParagEkbote Thanks for the PR. I'm worrying about the CI failure. All examples in this repository are continuously tested by the GitHub Actions, so I expect that this example can be tested too. It seems that the API key of the Comet is missing. Is is possible to generate the API key which can be used here?

@ParagEkbote
Copy link
Contributor Author

@ParagEkbote Thanks for the PR. I'm worrying about the CI failure. All examples in this repository are continuously tested by the GitHub Actions, so I expect that this example can be tested too. It seems that the API key of the Comet is missing. Is is possible to generate the API key which can be used here?

No, we will probably need to add it as a GitHub repo secret in Settings->Secrets and Variables->Actions. WDYT?

cc: @HideakiImamura

@HideakiImamura
Copy link
Member

Thanks for the reply. I understood.

Is it OK to use the generated API key of one of us (comitters) or should be use any universal API key?

@ParagEkbote
Copy link
Contributor Author

ParagEkbote commented Mar 28, 2025

Thanks for the reply. I understood.

Is it OK to use the generated API key of one of us (comitters) or should be use any universal API key?

Unless there is an enterprise plan available, we will need to use an individual API key.

cc: @HideakiImamura

@nzw0301
Copy link
Member

nzw0301 commented Mar 31, 2025

I'm not sure but how about https://www.comet.com/docs/v2/api-and-sdk/python-sdk/advanced/running-offline/ like WandB example

?

@HideakiImamura
Copy link
Member

I discussed with other committers. It would be great to remove the CI configuration file from this PR.

@ParagEkbote
Copy link
Contributor Author

I discussed with other committers. It would be great to remove the CI configuration file from this PR.

I have removed the CI config file. Could you please review?

cc: @HideakiImamura

@ParagEkbote ParagEkbote requested a review from nzw0301 March 31, 2025 08:01
Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

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

How about using the same objective and printing result code as
https://github.com/optuna/optuna-examples/blob/main/wandb/wandb_integration.py?

This is because it would be more easily understood without manually calling log_metric and similar code to other MLops code (namely, wandb here).

@nzw0301
Copy link
Member

nzw0301 commented Mar 31, 2025

Hi @HideakiImamura, thank you for directing us.
Do you mean this repo does not test this comet example similar to the Kubernetes example https://github.com/optuna/optuna-examples/tree/main/kubernetes or plan to add CI config by another pull request for simplicity?

@ParagEkbote
Copy link
Contributor Author

I have updated the example. Could you please review?

cc: @nzw0301

@ParagEkbote ParagEkbote requested a review from nzw0301 April 1, 2025 17:59
@nzw0301
Copy link
Member

nzw0301 commented Apr 2, 2025

Lovely, I will check the changes this weekend!

Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

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

Hi, I think the current script does not work correctly at

    comet_callback = CometCallback(project_name="comet-optuna-sklearn-example")

Can you fix this?

@ParagEkbote
Copy link
Contributor Author

I apologize for making breaking changes in my example. I've corrected it now and it shows the following Output:

Number of finished trials: 20

Best trial:
Value: 1.0

Params:
min_samples_leaf: 9
max_depth: 8
min_samples_split: 4

Could you please review?

cc: @nzw0301

@ParagEkbote ParagEkbote requested a review from nzw0301 April 6, 2025 14:31
Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

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

Lovely. Could you rename the python script from comet_callback.py to comet_integration.py?

@nzw0301
Copy link
Member

nzw0301 commented Apr 6, 2025

Do you mean this repo does not test this comet example similar to the Kubernetes example https://github.com/optuna/optuna-examples/tree/main/kubernetes or plan to add CI config by another pull request for simplicity?

I realised that offline evaluation without an API key might be impossible, so please ignore my careless comments. Sorry.

@ParagEkbote
Copy link
Contributor Author

I've renamed the file. Could you please review?

cc: @nzw0301

Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

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

sorry minor things.

@ParagEkbote
Copy link
Contributor Author

Are any pending changes that need to be addressed?

Could you please let me know?

cc:@HideakiImamura

@github-actions
Copy link

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Apr 13, 2025
@c-bata c-bata added enhancement Change that does not break compatibility and not affect public interfaces, but improves performance feature Change that does not break compatibility, but affects the public interfaces of examples. and removed stale Exempt from stale bot labeling. enhancement Change that does not break compatibility and not affect public interfaces, but improves performance labels Apr 14, 2025
@c-bata
Copy link
Member

c-bata commented Apr 14, 2025

Let me merge this PR since this PR already got approval from the maintainer.

@ParagEkbote Thank you for your contribution!

@c-bata c-bata merged commit 517087d into optuna:main Apr 14, 2025
1 check passed
@c-bata c-bata added this to the v4.4.0 milestone Apr 14, 2025
@ParagEkbote ParagEkbote deleted the Add-Comet-Example branch April 14, 2025 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Change that does not break compatibility, but affects the public interfaces of examples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add comet integration example

4 participants