Skip to content

Adding crossing minimisation and straightness model#136

Open
stratus85 wants to merge 15 commits intoricklupton:masterfrom
stratus85:optimise-harris
Open

Adding crossing minimisation and straightness model#136
stratus85 wants to merge 15 commits intoricklupton:masterfrom
stratus85:optimise-harris

Conversation

@stratus85
Copy link

with more_testing folder

stratus85 and others added 8 commits July 12, 2024 11:06
Now the `SankeyLayout` contains the node positions, which can be
optimised and passed into the `to_widget` method.  I think it makes
sense to keep this separate from the `SankeyData`.

Note FIXME comments for parts that are not fully implemented.
Edge weight should come from `SankeyLink.link_width`, not
`SankeyLink.data["value"]`: while "value" is the default it doesn't have
to be called that.

Fix attribs default value factories for `SankeyNode` and `SankeyLink`.

Add some xfail tests for diagram optimisation.
@ricklupton
Copy link
Owner

@stratus85 thanks for getting this started. I've tested and done some reorganisation to simplify the functions used to optimise the node order and node positions. Can you try the updated "test1" (now renamed to docs/cookbook/layout-optimisation.ipynb) to check it works for you and makes sense?

I've also added some unit tests in the file test/test_node_position_optimisation.py. You can run these by running the command pytest test/test_node_position_optimisation.py -v (or just run all the tests using pytest test). You should see one test passing and 3 others "xfail" which means "expected fail". These highlight cases that I think should be true but aren't currently accounted for by the code -- it would be great if you can have a think about how to make these tests pass, and we can discuss when we meet.

model = straightness_model(sankey_data)
# FIXME this needs to know what scale we want to use?
ys = optimise_position_model(model, scale, wslb=minimum_gap)
ys = optimise_position_model(model, scale, wslb=minimum_gap*scale)
Copy link
Owner

Choose a reason for hiding this comment

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

@stratus85 why does the minimum gap need to be scaled? I think it probably makes sense to have this defined in terms of pixels (i.e. the actual gap in the final Sankey diagram)?

{
"cell_type": "code",
"execution_count": null,
"execution_count": 1,
Copy link
Owner

Choose a reason for hiding this comment

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

@stratus85 not a big problem, but you can see that there are lots of changes in the notebook JSON file here which makes it hard to see the thing you actually changed. Please clear the notebook output and save before committing it to git.

1. Relative positioning of nodes in the layer
2. Used that to change positioning of nodes
3. Tried including extra punishment for not being completely straight (needs revision)
New global steel flows dataset, added dataset_manipulation.py and significantly changed diagram_optimisation.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants