search

automl / ConfigSpace

Domain specific language for configuration spaces in Python/Cython. Useful for hyperparameter optimization and algorithm configuration.
https://automl.github.io/ConfigSpace/
Other
193 stars 89 forks source link

Refactor: Remove Cython - Reduce tech debt #346

Open eddiebergman opened 6 months ago

eddiebergman commented 6 months ago

This PR attempts to first convert everything to Python, so a type checker can be run across everything as well as use linters properly. This also includes updating everything to pytest.

Note: This PR is unreviewable. The strategy has been to minimally change tests while refactoring to ensure tested behaviour still passes. Some notable exceptions are where functionality was removed or sampling strategy was changed and the expected values differ. Unfortunatly the formatter has rendered them also unreviewable.

Update: All SMAC unittests passed without code changes so it seems mostly backwards compatible.

Some notable changes:

TODO:

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 92.06843% with 51 lines in your changes missing coverage. Please review.

Project coverage is 78.91%. Comparing base (93abc5b) to head (2a29120). Report is 2 commits behind head on main.

:exclamation: Current head 2a29120 differs from pull request most recent head 09beefd

Please upload reports for the commit 09beefd to get more accurate results.

Files Patch % Lines
ConfigSpace/forbidden.py 82.35% 17 Missing and 1 partial :warning:
ConfigSpace/conditions.py 93.04% 7 Missing and 1 partial :warning:
ConfigSpace/configuration_space.py 64.28% 3 Missing and 2 partials :warning:
ConfigSpace/hyperparameters/normal_float.py 88.09% 4 Missing and 1 partial :warning:
ConfigSpace/hyperparameters/normal_integer.py 85.71% 4 Missing and 1 partial :warning:
ConfigSpace/hyperparameters/categorical.py 90.90% 3 Missing :warning:
ConfigSpace/hyperparameters/ordinal.py 91.42% 3 Missing :warning:
ConfigSpace/hyperparameters/uniform_integer.py 96.29% 2 Missing :warning:
ConfigSpace/hyperparameters/numerical.py 97.05% 1 Missing :warning:
ConfigSpace/hyperparameters/uniform_float.py 96.96% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #346 +/- ## ========================================== + Coverage 73.54% 78.91% +5.37% ========================================== Files 29 45 +16 Lines 2831 4767 +1936 Branches 629 1005 +376 ========================================== + Hits 2082 3762 +1680 - Misses 632 806 +174 - Partials 117 199 +82 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonovice commented 1 month ago

For everyone else being stuck due to type errors etc: You can pull in the current state of this PR to your poetry env with

poetry add git+https://github.com/automl/ConfigSpace.git@refs/pull/346/merge

Might help in your case.

eddiebergman commented 2 weeks ago

@mfeurer I converted all the documentation pages to use mkdocs-material format, i.e. markdown documentation. The primary reason is I know how to build docs/version and deploy them. It's also a much less finicky syntax, i.e. I never get a sphinx link correct.

I've documented most of the major modules at this point but have yet to set up the actual doc generation process. What I think would be the most useful for you to review at this point would be the pages under docs/reference.

I mostly left the documentation the same but I've added a clearer introduction to hyperparameters

Due to a no-longer-updated take on PCS format, I've now focused serialization on json() and yaml() which made this page change: https://github.com/automl/ConfigSpace/blob/typing-cython-3/docs/reference/serialization.md?plain=1 The encoder/decoder part is to satisfy this issue #357. It also enables us to allow people to create their own import and export formats as required for them.

The rest of the docs are mostly the same.

I still have left to do API documentation pass over ConfigurationSpace and Configuration as well as the utils.py file.

After that I need to setup mkdocs and fix any potential typoed links and such. The look and layout will be mostly similar to amltk docs.

Sorry for migration away from sphinx, but I really just don't like writing in it and it's hard to explain the nuances to anyone new.

sarah-segel commented 1 week ago

FYI: I tested running DeepCave with ConfigSpace 0.8.0 and ran into the following problems when executing :

1) I run into an error when calling from ConfigSpace.c_util import change_hp_value, check_forbidden 2) It seems like the default values in my configspace.json cause a problem when calling from ConfigSpace.read_and_write import json as cs_json cs_json.read(self.configspace_fn.read_text()) where I get: __init__() got an unexpected keyword argument 'default' 3) When calling hp._transform_scalar(value) I get AttributeError: 'UniformFloatHyperparameter' object has no attribute '_transform_scalar'

eddiebergman commented 1 week ago

@mfeurer, I did a major push on getting the documentation updated to mkdocs. You can view it locally by cloning this branch, pip install -e "[dev]" and running mkdocs serve. Likely another push should get this to the finish line.

@sarah-segel thanks for the report back, I will look into them with the next big push. Do you have any more information on the first one other than "ran into an error"? Second one is likely a bug on my behalf, will investigate. Last one is good to know about. It was a private method so I considered safe enough to remove. I will add it back in with the deprecation warning.

eddiebergman commented 3 days ago

@sarah-segel should have been fixed now, can you try again?