Ring search algorithms

[beesRawes0me]

No description provided.

[beesRawes0me]

There is a bug now returning just empty lists, I will look at it tomorrow.

[beesRawes0me]

Some things I'm unsure with:

• its quite a mess now, but I wasn't sure how to group the new added records/functions
• is it better to pattern match on the record or use the extractor functions in the utility functions?
• is it possible to implement semigroup path with on?
• I left rings as Integer since its not a path, is that okay?

[stefan-hoeck]

Some things I'm unsure with:

• its quite a mess now, but I wasn't sure how to group the new added records/functions

It's not too messy, and we can still move some of the types and utilities to the Graph.Indexed.Types module.

• is it better to pattern match on the record or use the extractor functions in the utility functions?

That can't be answerd in general. In case of Visited and Path it does not realy matter.

• is it possible to implement semigroup path with on?

Maybe, but that's not very useful here.

• I left rings as Integer since its not a path, is that okay?

Now that you ask, maybe we should have the followings: Ring k with the Semigroup you already defined, because this is definitely a valid representation of rings, and xor is a way to make new rings of existing ones. We should then rename Path k to something like PreRing and make it private (that's the default), because that's what our "paths" actually are: rings in the making. So, something like the following:

record PreRing k where

add : Fin k -> PreRing k -> PreRing k

inPreRing : Fin k -> PreRing k -> Bool

record Ring k where

inRing : Fin k -> Ring k -> Bool

Semigroup (Ring k) where

Monoid (Ring k) where

merge : PreRing k -> PreRing k -> Ring k -- use `xor` here as well

Ring k and Visited k should now go to Data.Graph.Indexed.Types. PreRing k should stay here as an implementation detail. As a result of our ring finder, we should then return a list of rings not the full state, because otherwise PreRing k would be part of an exported result and that does not make a lot of sense.

[beesRawes0me]

I moved Visited k and Ring k and their utility functions to Types.idr at the bottom of the file. The exported function search1 now returns a list of rings but getRings and getRings' still return the state.

[stefan-hoeck]

I moved Visited k and Ring k and their utility functions to Types.idr at the bottom of the file. The exported function search1 now returns a list of rings but getRings and getRings' still return the state.

Great! Looking good now. There are still a couple of layouting issues (have you read the style guide ;-) ), but it will be easier to quickly discuss those in person. I think, once you implement search for the whole - potentially disconnected - graph, search1 must again return the state, but it should then have private visibility. So that's the next step: Make this work for disconnected graphs. :-)

[stefan-hoeck]

I think, once you implement search for the whole - potentially disconnected - graph, search1 must again return the state, but it should then have private visibility. So that's the next step: Make this work for disconnected graphs. :-)

Wait, no, ignore the above about search1 returning the state again. It should just be renamed to search, that's all. It should still be able to handle disconnected graphs, though...

[beesRawes0me]

I moved Visited k and Ring k and their utility functions to Types.idr at the bottom of the file. The exported function search1 now returns a list of rings but getRings and getRings' still return the state.

Great! Looking good now. There are still a couple of layouting issues (have you read the style guide ;-) ), but it will be easier to quickly discuss those in person. I think, once you implement search for the whole - potentially disconnected - graph, search1 must again return the state, but it should then have private visibility. So that's the next step: Make this work for disconnected graphs. :-)

I have read it but apparently forgot some of it then :-)

[beesRawes0me]

implementation of Eq for PreRings is missing. I wasn't if and where I should implement this

[beesRawes0me]

I get a type error when using mkGraph on List (LNode () ) and List( LEdge () ) that I don't understand. If you have time maybe you could give me some feedback or we can look at it tomorrow :-)

[stefan-hoeck]

I get a type error when using mkGraph on List (LNode () ) and List( LEdge () ) that I don't understand. If you have time maybe you could give me some feedback or we can look at it tomorrow :-)

Look at the type of Data.Graph.Indexed.Util.mkGraph: It's first argument is not a list, nor do you need LNode and LEdge: These are from the other graph library we test against. The first argument of mkGraph is a vector of the desired length containing just the node labels at each position. The second argument is a list of edges with the correct index: Data.Graph.Indexed.Types.Edge.

[beesRawes0me]

Look good :-)

[beesRawes0me]

The algorithm detects ring systems with one common node as fused rings, which is not what we want right? It seems that the if statement to filter ring systems with more than 1 common node does not work the way I intended. I am not sure why.

[beesRawes0me]

I have a problem when typechecking test.ipkg that I dont know how to fix. I guess we can look at it this afternoon @stefan-hoeck?

[stefan-hoeck]

So, the bad news is: There was another bug. A few cycle bases (only 5 or so in CyBy) are too large. The good news: After some serious debugging, I found the issue - and it was again my own "cleverness"; so it is only fitting that I had to figure it out, haha.

Now, all cycle bases in all three test sets are of the correct size. Testing relevant cycle counts now.