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

Greedy colouring for decomposition trees of interval graphs and tests #34

Open
wants to merge 13 commits into
base: intervalDecomposition
Choose a base branch
from

Conversation

dia007
Copy link

@dia007 dia007 commented May 27, 2018

Changed the parameters and incorporated minor changes

@dia007 dia007 requested review from pdelvo, jiong-fu and PhoenixIra May 27, 2018 14:17
@PhoenixIra PhoenixIra added Coloring Graph Coloring Algorithms Tree Decomposition Tree Decomposition Algorithms labels May 28, 2018
Copy link

@PhoenixIra PhoenixIra left a comment

Choose a reason for hiding this comment

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

  • Could you fix the issue with the CI fail?
  • Could you also change your inputs to the decomposition as a Graph<Integer,E> and Map<Integer,Set>?
  • And some other more minor stuff

*/
protected Iterable<List<V>> getVertexOrdering()
{
System.out.println((Iterable<List<V>>) graph.vertexSet());

Choose a reason for hiding this comment

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

remove debug output

Copy link
Author

Choose a reason for hiding this comment

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

Done.

/**
* The input graph
*/
private Graph<List<V>, E> graph;

Choose a reason for hiding this comment

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

Currently we do not use List as the vertex set, since Lists are not really efficient for set structures and using empty lists for the leaves/roots would break the tree structure. Instead we use Graph<Integer, E> and Map<Integer,Set>. Could you change this in your implementation?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

for(List<V> outerVertex:getVertexOrdering() ) {

//the case of a generalised decomposition tree
/* for (E edge: graph.edgesOf(outerVertex)) {

Choose a reason for hiding this comment

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

Do you need this code? If not, remove it. If yes, remove it and if you will need it, use this commit to get the code.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you are right. I do not need it.

int maxColourAssigned = Collections.max(asssignedColors.values());


System.out.println(Arrays.asList(asssignedColors));

Choose a reason for hiding this comment

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

remove debug output

Coloring<Integer> coloring = new DecompositionTreeColour<>(newGraph).getColoring();
assertEquals(3, coloring.getNumberColors());
Map<Integer, Integer> colors = coloring.getColors();
assertEquals(0, colors.get(1).intValue());

Choose a reason for hiding this comment

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

You should not assume the color here. Instead test whether vertices differ in they color or have the same color, so that if in a future commit, the algorithm get changed and results another (valid i.e. by renaming) coloring, this tests will also accept the result.

Copy link
Author

Choose a reason for hiding this comment

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

Done. thank you.

Coloring<Integer> coloring = new DecompositionTreeColour<>(newGraph).getColoring();
assertEquals(3, coloring.getNumberColors());
Map<Integer, Integer> colors = coloring.getColors();
assertEquals(0, colors.get(1).intValue());

Choose a reason for hiding this comment

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

same as in line 87

@christophgruene
Copy link

It would be good to move this into the color package

@dia007 dia007 force-pushed the intervalDecomposition branch from b2406e8 to 3542b5a Compare June 17, 2018 15:00
@dia007 dia007 force-pushed the intervalDecomposition branch from 3542b5a to b46bb2a Compare August 10, 2018 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coloring Graph Coloring Algorithms Tree Decomposition Tree Decomposition Algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants