Refactor: Externalize Random Forest Parameters.#623
Conversation
|
@regulartim please review this PR when you get time |
greedybear/settings.py
Outdated
|
|
||
| DJANGO_LOG_DIRECTORY = "/var/log/greedybear/django" | ||
| ML_MODEL_DIRECTORY = os.path.join(BASE_DIR, "mlmodels/") # "/opt/deploy/greedybear/mlmodels" | ||
| ML_CONFIG_PATH = os.path.join(BASE_DIR, "configuration/ml_config.json") |
There was a problem hiding this comment.
Wouldn't "ML_CONFIG_FILE" be a better name?
There was a problem hiding this comment.
No, I meant something different: I think "ML_CONFIG_FILE" would be a better name for the constant. The file name "ml_config.json" is fine.
This reverts commit 21c7d67.
|
Looks good! 👍 In the checklist you marked that you have created tests. Where are they? |
Since this was a refactor to externalize the hyperparameters and all existing tests were passing, I interpreted the scope as small enough that specific new tests weren't critical at that stage. |
|
I was just confused that you checked the box but I couldn't see any tests. But it's good that you added some. It's always better to have new features covered by test cases. Good work. 👍 |
Description
This PR refactors the Random Forest implementation to externalize hyperparameters to a JSON configuration file (configuration/ml_config.json). Currently, these parameters are hardcoded in the RFClassifier and RFRegressor classes. Moving them to a configuration file allows for easier tuning and experimentation without modifying the codebase.
Related issues
Closes #614
Type of change
Checklist
develop.Black,Flake,Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.