Skip to content

Introduce UpdateFinishedTrialError to raise an error when attempting to modify a finished trial#6001

Merged
not522 merged 3 commits intooptuna:masterfrom
sawa3030:add/new-error
Mar 7, 2025
Merged

Introduce UpdateFinishedTrialError to raise an error when attempting to modify a finished trial#6001
not522 merged 3 commits intooptuna:masterfrom
sawa3030:add/new-error

Conversation

@sawa3030
Copy link
Copy Markdown
Collaborator

@sawa3030 sawa3030 commented Mar 4, 2025

Motivation

In the current implementation, modifying finished trials in Optuna raises a RuntimeError. However, RuntimeError is a generic exception that does not accurately reflect the nature of the issue. This PR introduces a more specific exception UpdateFinishedTrialError to improve clarity and maintainability.

Description of the changes

  • Introduced UpdateFinishedTrialError, a new exception that inherits from both RuntimeError and OptunaError.
  • Updated the following methods in BaseStorage to raise UpdateFinishedTrialError when attempting to modify a finished trial:
    • set_trial_param
    • set_trial_state_values
    • set_trial_user_attr
    • set_trial_system_attr
  • The error is raised using check_trial_is_updatable, except in JournalStorage, where trial updatability is checked within _trial_exists_and_updatable.
  • Update the tests

@sawa3030 sawa3030 marked this pull request as ready for review March 5, 2025 00:40
@nabenabe0928 nabenabe0928 assigned c-bata, nabenabe0928 and not522 and unassigned c-bata Mar 5, 2025
@nabenabe0928
Copy link
Copy Markdown
Contributor

@not522 Could you review this PR?

@not522
Copy link
Copy Markdown
Member

not522 commented Mar 5, 2025

Thank you for your PR! Could you update the files related to the error? As far as I know, the followings should be updated.

And, we also need to update exceptions.rst.

@sawa3030
Copy link
Copy Markdown
Collaborator Author

sawa3030 commented Mar 7, 2025

@nabenabe0928 Thank you for pointing that out. I've implemented the changes you suggested and thoroughly reviewed all parts where RuntimeError appears. Hopefully, I’ve made all the necessary updates. Please let me know if there's anything I missed!

Copy link
Copy Markdown
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

Could you check the document formats?

Co-authored-by: Naoto Mizuno <naotomizuno@preferred.jp>
Copy link
Copy Markdown
Contributor

@nabenabe0928 nabenabe0928 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, LGTM!
I also checked the documentations:)

@nabenabe0928 nabenabe0928 removed their assignment Mar 7, 2025
Copy link
Copy Markdown
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

LGTM!
Note: UpdateFinishedTrialError inherits from RuntimeError, so it should not affect user code.

@not522 not522 added this to the v4.3.0 milestone Mar 7, 2025
@not522 not522 added the code-fix Change that does not change the behavior, such as code refactoring. label Mar 7, 2025
@not522 not522 merged commit aef37e7 into optuna:master Mar 7, 2025
14 checks passed
@sawa3030 sawa3030 deleted the add/new-error branch November 14, 2025 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-fix Change that does not change the behavior, such as code refactoring.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants