-
Notifications
You must be signed in to change notification settings - Fork 249
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
Redefines fitness function for Max K-Color #39
base: master
Are you sure you want to change the base?
Conversation
Check that all neighbors are different color rather than same.
if state[self.edges[i][0]] == state[self.edges[i][1]]: | ||
for v, color in enumerate(state): | ||
# Check that all adjacent nodes are different color | ||
if all(state[neighbor] != color for neighbor in self.neighbors.get(v, set())): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another quote from Isbell's paper:
Here, we define Max K-Coloring to be the task of finding a coloring that minimizes the number of adjacent pairs colored the same
There's some question in my mind as to whether all
should be used here or if fitness
should be proportional to the number of edges for which this holds true. In either case, I noticed that there's a test failing because the fitness value here changed, so this probably shouldn't be merged right now either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @asantas93,
I think, as you said, the all
presents a problem here, since what we are interested in isn't whether all the neighbors are a different color from a given node, but how many adjacent pairs are colored the same (as stated in the Isbell quote that you provided). If you could find some way of rewriting line 935 of your code in order to account for that, then I think your code could be potentially a lot faster than the original code for this fitness function.
Regards,
Genevieve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @asantas93
Thanks for making this pull request. I think your update has the potential to make this fitness function a lot more efficient. However, as you pointed out, I think there is an issue with the all
in line 935. If you could fix that, then I will merge this with the code base.
Regards,
Genevieve.
if state[self.edges[i][0]] == state[self.edges[i][1]]: | ||
for v, color in enumerate(state): | ||
# Check that all adjacent nodes are different color | ||
if all(state[neighbor] != color for neighbor in self.neighbors.get(v, set())): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @asantas93,
I think, as you said, the all
presents a problem here, since what we are interested in isn't whether all the neighbors are a different color from a given node, but how many adjacent pairs are colored the same (as stated in the Isbell quote that you provided). If you could find some way of rewriting line 935 of your code in order to account for that, then I think your code could be potentially a lot faster than the original code for this fitness function.
Regards,
Genevieve.
Check that all neighbors are different color rather than same.
(from https://www.cc.gatech.edu/~isbell/tutorials/mimic-tutorial.pdf)