Skip to content

[feature] #1191 is trained property to indicate the state of the model. #1232

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

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

timruhkopf
Copy link
Collaborator

@timruhkopf timruhkopf commented Apr 17, 2025

@timruhkopf timruhkopf requested a review from Copilot April 17, 2025 06:27
@timruhkopf timruhkopf self-assigned this Apr 17, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an is_trained property across multiple model implementations and updates corresponding tests to assert the trained state of the models.

  • Adds assertions in tests for RandomForest, GP, and MCMC GP to verify is_trained before and after training
  • Modifies the training methods in RandomForest, Multi-objective models, Gaussian process (MCMC based), and AbstractModel to update the _is_trained flag
  • Updates the CHANGELOG to document the new trained property

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_model/test_rf.py Added assertions for is_trained in RandomForest tests
tests/test_model/test_gp_mcmc.py Added assertions for is_trained in GP_MCMC tests
tests/test_model/test_gp.py Added a dedicated test for is_trained in GP model
smac/model/random_forest/random_forest.py Sets _is_trained to True after fitting the forest
smac/model/multi_objective_model.py Implements a read-only is_trained property using all()
smac/model/gaussian_process/mcmc_gaussian_process.py Changes _is_trained assignment to depend on all model flags
smac/model/abstract_model.py Initializes _is_trained to False and exposes is_trained
CHANGELOG.md Documents the new is_trained property feature

Safeguard against the unlikely event that no model is in the MCMCGP,

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@timruhkopf timruhkopf requested review from benjamc and Copilot April 17, 2025 06:31
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an is_trained property to indicate whether models have been trained. Key changes include:

  • Adding tests in models’ test files (rf, gp, gp_mcmc) to verify the state of is_trained before and after training.
  • Updating model training methods in random_forest, multi_objective_model, mcmc_gaussian_process, and abstract_model to appropriately set the is_trained flag.
  • Documenting the new property in the CHANGELOG.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_model/test_rf.py Added assertions for is_trained before and after training of RandomForest model
tests/test_model/test_gp_mcmc.py Added assertions for is_trained for the GP MCMC model training
tests/test_model/test_gp.py Added a new test to validate the is_trained property in the GP model
smac/model/random_forest/random_forest.py Sets the is_trained flag to True after fitting the random forest model
smac/model/multi_objective_model.py Introduces is_trained property and sets it based on the training state of internal models
smac/model/gaussian_process/mcmc_gaussian_process.py Updates is_trained flag setting with a condition based on the presence of internal models
smac/model/abstract_model.py Initializes the is_trained flag and exposes it via a property
CHANGELOG.md Updates the changelog to include the new is_trained feature

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

Successfully merging this pull request may close these issues.

1 participant