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

Redefines fitness function for Max K-Color #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions mlrose/fitness.py
Original file line number Diff line number Diff line change
Expand Up @@ -904,9 +904,14 @@ class MaxKColor:
def __init__(self, edges):

# Remove any duplicates from list
edges = list({tuple(sorted(edge)) for edge in edges})

self.edges = edges
self.neighbors = {}
for v1, v2 in edges:
if v1 not in self.neighbors:
self.neighbors[v1] = set()
if v2 not in self.neighbors:
self.neighbors[v2] = set()
self.neighbors[v1].add(v2)
self.neighbors[v2].add(v1)
self.prob_type = 'discrete'

def evaluate(self, state):
Expand All @@ -925,9 +930,9 @@ def evaluate(self, state):

fitness = 0

for i in range(len(self.edges)):
# Check for adjacent nodes of the same color
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())):
Copy link
Author

@asantas93 asantas93 Oct 8, 2019

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.

Copy link
Owner

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.

fitness += 1

return fitness
Expand Down