Skip to content
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

129 add bayesian optimization #137

Merged
merged 18 commits into from
Aug 15, 2024
Merged

129 add bayesian optimization #137

merged 18 commits into from
Aug 15, 2024

Conversation

jkriwet
Copy link
Contributor

@jkriwet jkriwet commented Jul 15, 2024

Closes #129

@jkriwet jkriwet marked this pull request as ready for review July 15, 2024 09:19
@jkriwet jkriwet requested a review from FWuellhorst July 15, 2024 09:19
Copy link
Collaborator

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

@jkriwet Thanks for the feature!
Can you further:

  • include the option in the example for optimization?
  • Increase the version number to 0.5.0 due to the new feature? Also add the changelog.

@FWuellhorst
Copy link
Collaborator

@jkriwet : When doing the review, can you add the following kwarg?

"""gp = None"""
gp = kwargs.get("gp", None)
if gp is not None:
    optimizer._gp = gp

@jkriwet
Copy link
Contributor Author

jkriwet commented Aug 14, 2024

@jkriwet Thanks for the feature! Can you further:

  • include the option in the example for optimization?
  • Increase the version number to 0.5.0 due to the new feature? Also add the changelog.

Done. Also changed example as discussed internally

@jkriwet
Copy link
Contributor Author

jkriwet commented Aug 14, 2024

@jkriwet : When doing the review, can you add the following kwarg?

"""gp = None"""
gp = kwargs.get("gp", None)
if gp is not None:
    optimizer._gp = gp

Done

@jkriwet jkriwet requested a review from FWuellhorst August 14, 2024 11:07
setup.py Outdated
@@ -13,6 +13,7 @@
'openpyxl>=3.0.5',
'xlrd>=2.0.1',
'pymoo==0.5.0',
'bayesian-optimization==1.5.1',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why different version then in requirements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to version in the requirements. 1.5.1 works, but not for python version <= 3.9 (or 3.8). Since nothing important is changed in 1.5.1, fixing to 1.4.3 for now

@FWuellhorst
Copy link
Collaborator

@jkriwet : Looks really nice the new feature and example! I further added markers to see which point is where.
pymoo seems to have resolved the issue in their latest version, but removed all factories to avoid development efforts for them. This would mean we have to maintain a dict for string-class relation of pymoo objects, which I would not do to avoid overhead.
If the new pymoo version is of interest, we could remove the option to pass a string and just have users import the class beforehand.

@jkriwet
Copy link
Contributor Author

jkriwet commented Aug 15, 2024

@FWuellhorst Thank you! Since the cma imports seemed to still make problems i have now changed the algorithmn imports and pulled them from pymoo to ebcpy.

In pymoo, when choosing the algorithm over the get_algorithms() function, all algorithms are imported. Only the CMAES algorithm makes problems with the cma import. I have deleted that as an import (and algorithm choice) for ebcpy and now everything works, also in the CI for python >= 3.10.

I do think the string-class relation is way more convenient and therefore dont see any reason as of yet to chang to the new pymoo version.

@jkriwet jkriwet merged commit fe992cd into master Aug 15, 2024
1 check passed
@jkriwet jkriwet deleted the 129-add-bayesian-optimization branch August 15, 2024 09:20
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.

add bayesisan optimization
2 participants