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

Fix wrong defaults of SteadyStateGA #87

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

engintoklu
Copy link
Collaborator

While initializing SteadyStateGA, if the operators argument was not given (or was given as an empty list) and if re_evaluate was given as True (which is the default), SteadyStateGA was making the wrong default assumption that the parent solutions do not have to be evaluated before the children. This wrong assumption caused the tournament selection phases of the cross-over operators to fail, as they encountered unevaluated parents.

This pull request fixes this wrong assumption. When operators is empty (which means that the operators are to be added later via use(...) method), and re_evaluate is True, the default assumption of SteadyStateGA will now be that the parents need to be evaluated first.

While initializing `SteadyStateGA`, if the
`operators` argument was not given (or was given
as an empty list) and if `re_evaluate` was given
as True (which is the default), `SteadyStateGA`
was making the wrong default assumption that the
parent solutions do not have to be evaluated
before the children. This wrong assumption
caused the tournament selection phases of the
cross-over operators to fail, as they encountered
unevaluated parents.

This commit fixes this wrong assumption.
When `operators` is empty (which means that the
operators are to be added later via `use(...)`
method), and `re_evaluate` is True, then
`SteadyStateGA` will by default assume that the
parents need to be evaluated first.
@engintoklu engintoklu added the bug Something isn't working label Aug 4, 2023
@engintoklu engintoklu self-assigned this Aug 4, 2023
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #87 (5b0db91) into master (9d31d59) will decrease coverage by 0.04%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
- Coverage   77.83%   77.80%   -0.04%     
==========================================
  Files          49       49              
  Lines        7332     7334       +2     
==========================================
- Hits         5707     5706       -1     
- Misses       1625     1628       +3     
Files Changed Coverage Δ
src/evotorch/algorithms/ga.py 66.66% <50.00%> (-0.19%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Collaborator

@NaturalGradient NaturalGradient left a comment

Choose a reason for hiding this comment

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

looks sensible :)

@Higgcz Higgcz merged commit c023f5d into master Oct 2, 2023
4 checks passed
@Higgcz Higgcz deleted the fix/steady-state-ga-eval-parents branch October 2, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants