neo4j / graph-data-science

Source code for the Neo4j Graph Data Science library of graph algorithms.
https://neo4j.com/docs/graph-data-science/current/
Other
621 stars 160 forks source link

`gds.alpha.collapsePath.mutate` does not consider `nodeLabels()` filter #194

Closed MattyCrowther closed 2 years ago

MattyCrowther commented 2 years ago

Describe the bug Hi I am unsure if this is expected behaviour or not as the documentation for this procedure is slim and inspecting projected graphs is tricky. In short, should the procedure (collapsePath) remove the traversal path? Using the example on the documentation: `CREATE (Dan:Person), (Annie:Person), (Matt:Person), (Jeff:Person),

(Guitar:Instrument), (Flute:Instrument),

(Dan)-[:PLAYS]->(Guitar), (Annie)-[:PLAYS]->(Guitar),

(Matt)-[:PLAYS]->(Flute), (Jeff)-[:PLAYS]->(Flute) ` Added 6 labels, created 6 nodes, created 4 relationships (EXPECTED)

CALL gds.graph.project( 'persons', ['Person', 'Instrument'], { PLAYS: { type: 'PLAYS', orientation: 'NATURAL' }, PLAYED_BY: { type: 'PLAYS', orientation: 'REVERSE' } }) NodeCount: 6, relationshipCount: 8 (EXPECTED)

CALL gds.alpha.collapsePath.mutate( 'persons', { relationshipTypes: ['PLAYS', 'PLAYED_BY'], allowSelfLoops: false, mutateRelationshipType: 'PLAYS_SAME_INSTRUMENT' } ) YIELD relationshipsWritten relsWritten: 4 (EXPECTED)

CALL gds.graph.list() Yield graphName, database, configuration, nodeCount, relationshipCount, schema, degreeDistribution, density, creationTime, modificationTime, sizeInBytes, memoryUsage RETURN graphName, database, configuration, nodeCount, relationshipCount, schema, degreeDistribution, density, creationTime, modificationTime, sizeInBytes, memoryUsage ORDER BY graphName ASC

nodeCount: 6 ,relsCount: 12 I assume these are the original relationships + the aggregated relationships. Is this expected behaviour I.e. should we then project this projection pulling out the aggregated graph we just created or is it an error?

To Reproduce Follow steps here: https://neo4j.com/docs/graph-data-science/current/alpha-algorithms/collapse-path/

GDS version: 2.02 Neo4j version: 4.46 Operating system: N/A

Mats-SX commented 2 years ago

Hello @MattyCrowther. I'm not sure why you closed the issue again; perhaps you arrived at an answer to your question on your own.

For the record, the intended effect of collapsePath is to be only additive. So I would answer your question

should the procedure (collapsePath) remove the traversal path?

in the negative: no, it should not remove the path, and not anything else either.

The intended usage pattern would be to use a relationship filter in a subsequent algorithm procedure call. The configuration parameter relationshipTypes is used for this, see documentation here.

For example, you might call WCC like this:

CALL gds.wcc.stream('persons', {relationshipTypes: ['PLAYS_SAME_INSTRUMENT']}

and then only the (4) collapsed paths would be input to the algorithm, and the other (8) relationships would not.

Hope this is helpful! Mats

MattyCrowther commented 2 years ago

Hi, I did yes I realised it is inherent to the step-change approach GDS takes. Thanks for the reply.

Just another quick question. Is the general parameter "nodeLabels" implemented for this? CALL gds.alpha.collapsePath.mutate( 'wwfrgbaorg', { relationshipTypes: ['http://www.nv_ontology.org/repressor', 'http://www.nv_ontology.org/repressed', 'http://www.nv_ontology.org/activator', 'http://www.nv_ontology.org/activated', 'http://www.nv_ontology.org/template', 'http://www.nv_ontology.org/product'], mutateRelationshipType: 'http://www.nv_ontology.org/Repression/tetR_p/CI_p', allowSelfLoops: false ,nodeLabels: ['http://shortbol.org/v2#tetR_p/1'] } ) YIELD relationshipsWritten Out: relationshipsWritten : 5 Its hard to introspect projected graphs but I think that the collapse is occurring from multiple source nodes because I would expect only one relationship to be written (from http://shortbol.org/v2#tetR_p/1) .

Mats-SX commented 2 years ago

@MattyCrowther No that does look like a bug. I will reopen this issue and make it about fixing that bug.

jjaderberg commented 2 years ago

Fixed by 709e010f Closing Thanks for reporting!