-
Notifications
You must be signed in to change notification settings - Fork 0
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
WIP: Graph display #53
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind the RFC template as it stands, although some of the language (e.g. "Dashboards") is too project-y for my taste!
- *including code examples if you're proposing an interface or system contract.* | ||
- *linking to project briefs or wireframes that are relevant.* | ||
|
||
## 4 Metrics & Dashboards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Dashboards"? I think measurable metrics is fine, and all we need to capture the product success / scope.
- It is difficult for users to create complicated layers without | ||
outside help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It is difficult for users to create complicated layers without | |
outside help. | |
- It is impossible to create complex workflows in the current linear layout, hindering the development and use of Dissolve for advanced scientific analysis. |
activities of a layer. Each module much be read in sequence and a | ||
mental modal of the entire, possibly non-linear, process must be | ||
constructed from this. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Usage relationships between data / objects are "weak", leading to significant data-management within the code in order to maintain a valid state. |
the modules as individual building blocks. | ||
- One glance at the workflow gives an overview of all of the modules | ||
and their relations | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Links between data / objects is well-defined and strong, removing data management issues almost completely
signals from the interface. The `createNode` slot takes a string that | ||
corresponds to the `type` property of the Node Interface and appends a | ||
new Node of the correct type. Correspondingly, `deleteNode` takes the | ||
index of a node and removes that node from the model. In the same |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be wise to continue demanding unique names for nodes / modules, so we don't need to access by index
and could use the object name instead.
This could have a significant impact on the structure of the input | ||
file, since it represents a major change in the state of the data. | ||
Furthermore, this may be a significant break change, as it would not | ||
be a trivial matter to convert existing, linear code into the new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This work would really represent a huge change - Dissolve 2 - and one which would break basically every existing input file. It might be possible to write some kind of importer for 1.X TOML input, but it may be difficult and perhaps not worth the effort. I would be happy to accept the fact that Dissolve 2 is a new starting point for everyone's simulations. One might argue there is little value to the user in importing an old simulation, since they will need to get used to the new workflows we'll be presenting.
|
||
This first method is to simply forbid invalid graphs. The model | ||
should reject any attempt to create invalid edges or delete nodes that | ||
are required. This had advantages in design and implementation, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are required. This had advantages in design and implementation, but | |
are required. This has advantages in design and implementation, but |
places limits on what can provide a good user experience. For | ||
example, every possible node input must have a **valid** default | ||
value, so that the connection to that input can be deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that strictly disallowing connections between dissimilar input / output types is the best route to go - something like Blender will allow you to connect related type inputs/outputs and still yield sensible results, but I don't think we need that softness here. Regarding the need for default values, this is what we currently have with keywords in modules and nodes, so not a problem moving forward.
`AbstractItemModel`. I chose `AbstractListModel` since it is the | ||
easier to subclass, but all of these models have an element of | ||
ordering that we currently do not care about. If we do care in the | ||
future, it might be better to have used `AbstractListModel`, but that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you meant either AbstractTableModel
or AbstractItemModel
here?
### 8.2 Underlying iteration model | ||
|
||
|
||
Do we truly want an `AbstractListModel` for the `edges` and `nodes`? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we do, at least based on how I'm understanding your proposal here as it stands. Creating this as a light framework feels like the best option, with the specific functionality we need only.
This is a rough work in progress on a description of the graph library. I'm only pushing it at this point to confirm that the diagrams render correctly.