Skip to content

Add timeline determination and validation logic#27

Closed
maarten-ic wants to merge 18 commits into
developfrom
feature/timelines
Closed

Add timeline determination and validation logic#27
maarten-ic wants to merge 18 commits into
developfrom
feature/timelines

Conversation

@maarten-ic

@maarten-ic maarten-ic commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

This PR introduces a timeline determination and static checker to ymmsl.v0_2.

TODO:

  • Check which parts of the TimelineTree / TimelineNode API should be public (based on the needs of ymmsl2svg)
  • Improve error messages
  • Add documentation

The main idea behind this PR is that all timelines in a model form a tree (the TimelineTree) with the root timeline (":") as root of the tree.

Each component "lives" in a certain timeline, which is also the timeline of their F_INIT and O_F ports. O_I and S ports belong to a sub-timeline of the component's timeline: <component_timeline>:<port_timeline> (according to the timeline annotation in the yMMSL ports definition).

What this PR does:

  1. Introduce the TimelineTree object, with a TimelineNode for each timeline in the model.
  2. Add logic to determine the timeline for each component in the model. This may fail if:
    • Components are wired circularly: this is the case when a message on a component's F_INIT port depends in some way on the output produced by this component.
    • Components are wired in such a way that the timelines of incoming F_INIT messages don't agree (disregarding repeater filters).
    • A conduit (connected to an F_INIT port) has too many reducer filters. For example, you cannot add a last conduit filter when the sender is an O_F port in the root timeline.
  3. Add a function for additional consistency checks: TimelineTree.check_consistent(). This ensures that all conduits connect to the right timelines. This is more extensive than the consistency during the previous pass:
    • Also check conduits that are sending to an S port. Step 2. only considers conduits connecting to F_INIT ports.
    • Check that conduits with repeater filters are consistent with the other conduits connecting to the F_INIT ports.

Some things to discuss (perhaps):

  1. Do you agree with the TimelineTree and TimelineNode class names?
  2. I'm not very happy with the module name ymmsl.v0_2.timelines yet, since it feels like I should find the Timeline object here (but instead it's TimelineTree/Node). What do you think?
  3. Right now I'm so into the details of the timeline functionality that I find it hard to judge if the error messages are clear (enough) to the average user. Do you think this is a good level of detail/explanation to start with?
    N.B. I've put the error messages and logic for providing details in separate Error classes, to separate that as much as possible from the logic checks themselves.

- Make most TimelineTree variables "private". Accessor functions to be created for public API.
- Add tests
- `X | None` is not supported in py3.9 -> replace with `Optional`
- Fix flake8 issues
- Add check_consistent() to TimelineTree to check timeline consistency of all conduits
- Keep track of "parent" components that have O_I/S ports sending in a particular timeline
- Additional public API properties for TimelineNode
@LourensVeen

Copy link
Copy Markdown
Contributor

I've looked at this and thought about it a bit, and I have some preliminary comments and answers. To be discussed:

  • The two-phase approach makes sense to me, goes along the lines of my original ideas so hard to complain 😄. It looks correct, though I haven't split all the hairs yet.

  • I'd have done the recursion in TimelineTree._assign_component a bit more Dijkstra-like (find components with no incoming F_INIT ports to start with, process those, then recursively find and add components with incoming F_INIT ports from ones we've already processed, until they're all done), but it's correct as far as I can tell and readable enough.

  • I love the specific exceptions, makes the code cleaner and makes it easier to use them programmatically. We should have more of this.

Provisional answers:

  1. I think that what is now v0_2.ports.Timeline should become TimelineReference, so that TimelineNode can become Timeline, and TimelineTree should be TimelineChecker since it's really a class that does stuff rather than representing something. Or maybe a bit more abstractly Timelines because it does do a little of the latter it seems.
  2. We have Move consistency checks to separate module? #43 which suggests moving the consistency checks currently on ymmsl.v0_2.Configuration to a separate module. What if we made a ymmsl.v0_2.validation.consistency module with a ConsistencyChecker class with those checks, and a ymmsl.v0_2.validation.timelines with the Timeline and TimelineChecker classes?
  3. This looks nice! The only thing I can think of is to maybe add a bit more detail in CyclicDependency. People often want to do that (with some ugly hack to break the cyclic dependency) and then get confused that MUSCLE3 doesn't allow it. Could be a link to an explanation and work-around in the docs.

@maarten-ic

Copy link
Copy Markdown
Collaborator Author
  • Put determined timelines back on Component (absolute timelines) and Port (relative to Component?) objects
  • Update timeline annotations when importing/resolving (components are renamed to x.y.<component> and timeline annotations should as well to prevent clashes)
  • Move to v0_2.validation.timelines. Function: resolve_timelines() because it updates the timelines on the data objects
  • Cyclic dependency documentation (move to issue?)

@maarten-ic maarten-ic mentioned this pull request Jul 16, 2026
4 tasks
@maarten-ic

Copy link
Copy Markdown
Collaborator Author

This pull request is superseded by #62

@maarten-ic maarten-ic closed this Jul 17, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in 2026 MUSCLE3 improvements Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants