Analytical_Engine adding graph methods

[rolyhudson]

NOTE: Depends on

BHoM/BHoM#1036

Issues addressed by this PR

Prototype methods for use with refactored analytical graph and new topological objects.

Test files

https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/01_Test%20Scripts/BHoM_Engine/Analytical_Engine/Issues/GraphProtoype?csf=1&web=1&e=14PXm5
Link to folder containing tests / demos to show graph construction and querying

• Duct_Cable_Routing.gh to show 3 graph construction strategies and use of AStarShortest path
• RandomSpatialGraphs.gh demos construction of random spatial graphs from any object implementing INode. Intended as basis for testing methods.
• FromCurves.gh demos the creation of a graph from a collection of ICurves.
• OSMGraphMashUp2.gh demonstrates use of street geometry from OpenStreetMap to create a graph and then route along streets with Astar search.

Changelog

Additional comments

[IsakNaslundBh]

Some initial comments on some references

[IsakNaslundBh]

Diffing engine should be referenced via project, not dll

[IsakNaslundBh]

Suggested change

	<HintPath>ProgramData\BHoM\Assemblies\Diffing_oM.dll</HintPath>
	<HintPath>C:\ProgramData\BHoM\Assemblies\Diffing_oM.dll</HintPath>

[bhombot-ci]

Please be advised that the check with reference 1353086365 has more than 50 annotations of notes. API limitations restrict annotations to 50. You may need to rerun this check to obtain the next set when you make changes. At the time of reporting this check, there are 1758 additional annotations waiting

[rolyhudson]

@BHoMBot fix copyright

[bhombot-ci]

@rolyhudson sorry, I didn't understand.
Was that comment an instruction for me? If so, could you state again what check you would like me to do?
For example, if you would like me to fix the CSProject file(s) comment:

"@BHoMBot fix project file"

[rolyhudson]

@BHoMBot check project compliance

[bhombot-ci]

@rolyhudson to confirm, check-project-compliance task is now queued.

[bhombot-ci]

@rolyhudson just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @rolyhudson on BHoM

[rolyhudson]

@al-fisher + @adecler this is ready for a review now. Made changes as discussed and had a length back and forth with BHoMBot on compliance. 😆 Sorry to all who get all the comments.

[adecler]

Looking great overall.

Just some minor comments on the code. I refrained from making quite a few of them as they were mainly an opinion on the way I would implement something and not a requirement for the code to work. I still couldn't resist to make a few 😄 . I have also noticed a few parts of the code that seems to be work in progress, is that correct ? Not a big issue to me as all this code is new anyways.

A few additional comments from the use of your example scripts:

• have a create method for relation that takes in BHomObjects instead of Guids. You already provide that approach in a few other places like so would be nice to be consistent in helping the user.
• The Create.Graph Complain about lack of HashFragment when building Graph
• How come we have the same number of relations regardless of ducts being entities or not ? The relations are either between pairs of nodes (and duct potentially in sub-graphs) or relations are between duct and nodes. Otherwise, what's the point of having the ducts in the graph since they have no relation with anything ?

[rolyhudson]

• have a create method for relation that takes in BHomObjects instead of Guids. You already provide that approach in a few other places like so would be nice to be consistent in helping the user.
• The Create.Graph Complain about lack of HashFragment when building Graph
• How come we have the same number of relations regardless of ducts being entities or not ? The relations are either between pairs of nodes (and duct potentially in sub-graphs) or relations are between duct and nodes. Otherwise, what's the point of having the ducts in the graph since they have no relation with anything ?

@adecler thanks for your comments. On the above points.

• Added a method for relation with IBHoMObjects as inputs
• Now only attempting to remove the Hash Fragment if one exists
• Good question see below.

The last point should be on the table for for discussion in tomorrows session plus your other unwritten points.

Below are a few notes for that discussion:

I understand your third point refers to a test script where a duct 'owned' a dependency fragment referencing the guids of its start node and end node. A Relation was created that ignored the duct. This is redundant.
What I think we need is DependencyFragments that define the relations between the entity that owns the fragment and the entity(s) it shares relations with. So specificing both Source and Target in a DependencyFragment is not necessary.

I have been back through the logic of the mapping of DependencyFragment to Relation and made small changes that I think will help ensure the usage is clearer. This also is a response to BHoM/BHoM#1036 (comment)

Some considerations:

1. First the mapping is not direct. A single DependencyFragment does not map to a single Relation. In the previous version of the code I hade made this assumption, which I now believe to be incorrect and confusing.
2. When using the IDependencyFragments an entity (any IBHoMObject) can "own" a fragment.
3. This fragment indicates a Relation in the graph. The entity that owns the fragment should either be the Source or Target.
4. If the entity that owns the fragment is a Source it may have multiple targets.
5. If the entity that owns the fragment is a Target it may have multiple sources, (like the incoming nodes situation in GraphFlow).

To support the above we have two basic DependencyFragments intended for adding to a entity:

• SourcesDependencyFragment
• TargetsDependencyFragment

This is not exhaustive there will probably be more...

Both DependencyFragment include properties defining one or more guids for sources or targets.
Converting to relation can return a collection of Relations.
Looking forward to refining this further when we speak - with some graphical support.

[rolyhudson]

@adecler @alelom @al-fisher
A note to indicate changes discussed in our conversation earlier have been completed.

One exception is the suggested explicit cast from ILink to Relation. My understanding is that it is not possible to implement user defined cast from an Interface? Please correct me if this is wrong or if I misunderstood the suggestion - I could not see examples of this elsewhere in BHoM.

[adecler]

Looks good to me. Happy to merge at this stage.

[adecler]

One exception is the suggested explicit cast from ILink to Relation. My understanding is that it is not possible to implement user defined cast from an Interface? Please correct me if this is wrong or if I misunderstood the suggestion - I could not see examples of this elsewhere in BHoM.

Yeah, I didn't think of that when I suggested it yesterday. That's a shame but not the end of the world. We might have our own implicit casting system at the level of the UI at one point to solve that. Shouldn't be too hard to put in place but will have to wait a bit.

[bhombot-ci]

@rolyhudson just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @rolyhudson on BHoM

[rolyhudson]

/azp run BHoM_Engine.CheckInstaller

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[bhombot-ci]

@rolyhudson just to let you know, I have provided a check-installer result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @rolyhudson on BHoM