Feature/resolve timelines by maarten-ic · Pull Request #62 · multiscale/ymmsl-python

maarten-ic · 2026-07-16T12:22:46Z

Continuation of PR #27

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?)

Signature accepts a Sequence, but the logic previously only worked when providing a list. Other sequences silently didn't work.

Add `parent` property and `relative_to` (inspired by pathlib.Path)

maarten-ic · 2026-07-16T14:27:58Z

~~Update timeline annotations when importing/resolving (components are renamed to x.y.<component> and timeline annotations should as well to prevent clashes)~~

N.B. Resolving imports doesn't rename components (only models) and therefore shouldn't rename timelines.
~~Timelines should be renamed when flattening, but that logic resides in muscle3 so it's not part of this PR.~~

Edit: thinking a bit more about this, I'm not sure if we must rename timelines when flattening the hierarchical models:

Timelines that are exported on a model port must not change names
Timelines that are internal to a model will never interact with components outside the model and so this is not a problem for the logic in this PR if they have the same name

Include information on the conduits instead of just the components. Example error message: > Detected a dependency cycle in model 'cycle'. The component 'first' has an F_INIT port that depends on data produced by one of its own O_F or O_I ports: Conduit(first.final -> second.init) -> Conduit(second.final -> third.init) -> Conduit(third.final -> first.init). You may have an error in the conduits or may need a different coupling schema.

maarten-ic · 2026-07-17T10:16:55Z

N.B. I've improved the cyclic dependency error, hopefully this is clear enough without writing and pointing to a new documentation page.

…melines

LourensVeen · 2026-07-20T08:52:17Z

Edit: thinking a bit more about this, I'm not sure if we must rename timelines when flattening the hierarchical models:

Imagine a top-level model with two components a and b, with a dispatching to b. Both a and b are implemented using a submodel, and each of those submodels has a macro-micro model inside it with components named macro and micro, and with macro's F_INIT and O_F connected to model ports.

After flattening, we have a flat model with components a.macro, a.micro, b.macro and b.micro. Component a.micro is on timeline :a.macro and component b.micro is on timeline :b.macro. Without renaming during flattening, they'd both be on :macro and would appear to be on the same timeline while they're really not.

Timelines that are exported on a model port must not change names
Timelines that are internal to a model will never interact with components outside the model and so this is not a problem for the logic in this PR if they have the same name

Timelines on ports are usually relative timelines, and those shouldn't be a problem as long as the absolute timeline they're appended to is correct. They can interact with components outside the model just fine though, we can have a component macro inside of a submodel that has its O_I and S ports connected to O_I and S ports on the submodel, and a micro component in another submodel.

LourensVeen

Definitely headed in the right direction, nice work!

LourensVeen · 2026-07-20T09:35:03Z

+          o_f: final
+    ports:
+      f_init: init
+      timeline macro:meso:


Oh, that final colon could be confusing. I never considered that. Not for this PR, but would it make sense to quote the timeline to clarify that?

timeline "macro:meso":

Hmm, not sure. I'd expect that for the majority of cases you'll have no nested timelines, and then I like the unquoted version better.

I was actually not confused when writing this file: The syntax highlighting of my editor made it clear that the final colon is not part of the key, see screenshot:

LourensVeen · 2026-07-20T09:44:07Z

+    ConduitTimelineError,
+    CyclicDependency,
+    InconsistentTimelines,
+    TooManyReducerFilters,


Can we have too many repeaters as well? If I connect an init component to a micro with two repeaters? Or would that come out as InconsistentTimelines? But there aren't two known timelines in this case, so that's not what I would expect...

That would indeed come out as InconsistentTimelines (which then suggests to add a reducer, or remove a repeater).

Note that TooManyReducerFilters only triggers when trying to reduce "above" the root timeline. You can also have too many reducers and end up with an InconsistentTimelines error (which will suggest to remove a reducer or add a repeater).

Would it make sense to rename this exception to ReducerFilterOnRootTimeline (or similar) to avoid this confusion?

LourensVeen · 2026-07-20T09:57:27Z

+      second.final: third.init
+      third.final: first.init
+
+  reducer:


Can we have this also without the connection from first.o_f to second.f_init? That should work too, but does it?

Yes, we can, and it works 😄

I've added a testcase for this.

LourensVeen · 2026-07-20T10:20:15Z

+    """Determine timelines for each component and their O_I and S ports in this model.
+
+    An exception will be raised when the model timelines are not consistent. This
+    function updates the timeline attributes of the components and ports in the model.


It would be useful for people building tools on top to know exactly which exceptions are raised under which circumstances.

Good point, I've updated the documentation to point to the exception classes. The docstring of the exceptions explain the cases where they're raised.

Based on PR feedback

maarten-ic · 2026-07-21T12:39:28Z

Edit: thinking a bit more about this, I'm not sure if we must rename timelines when flattening the hierarchical models:

Imagine a top-level model with two components a and b, with a dispatching to b. Both a and b are implemented using a submodel, and each of those submodels has a macro-micro model inside it with components named macro and micro, and with macro's F_INIT and O_F connected to model ports.

After flattening, we have a flat model with components a.macro, a.micro, b.macro and b.micro. Component a.micro is on timeline :a.macro and component b.micro is on timeline :b.macro. Without renaming during flattening, they'd both be on :macro and would appear to be on the same timeline while they're really not.

Yes, fully agreed! For complete correctness you'd have to rename these internal timelines.

My point (which I didn't describe clearly, sorry for that!) was that timeline renaming isn't required for the checks in this PR. The flattened model is more of an internal MUSCLE3 state and I expect it's not required to have the renaming functionality in to get a properly behaving simulation. We'll have to test though 🙂

maarten-ic added 6 commits July 15, 2026 13:13

Additional dunder methods for Ports and Timeline

3d1f169

Fix potential typing issue

40d6bef

Signature accepts a Sequence, but the logic previously only worked when providing a list. Other sequences silently didn't work.

Extend Timeline API

56ebfb2

Add `parent` property and `relative_to` (inspired by pathlib.Path)

Add timeline checker and testcases

a093eca

Update public API in v0.2/__init__.py

7a1a4e1

Rename test module

016a815

maarten-ic added this to 2026 MUSCLE3 improvements Jul 16, 2026

github-project-automation Bot moved this to Todo in 2026 MUSCLE3 improvements Jul 16, 2026

maarten-ic moved this from Todo to In Progress in 2026 MUSCLE3 improvements Jul 16, 2026

maarten-ic marked this pull request as ready for review July 17, 2026 10:15

maarten-ic requested review from IrisvdWerf and LourensVeen July 17, 2026 10:15

maarten-ic mentioned this pull request Jul 17, 2026

Add timeline determination and validation logic #27

Closed

3 tasks

maarten-ic added 3 commits July 17, 2026 13:41

Fix docstring link

f98cc6b

Merge remote-tracking branch 'origin/develop' into feature/resolve-ti…

c354722

…melines

Update timeline annotation in test

78280ff

LourensVeen requested changes Jul 20, 2026

View reviewed changes

Improve exceptions, docstrings and add testcase

7f7add8

Based on PR feedback

Uh oh!

Conversation

maarten-ic commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maarten-ic commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maarten-ic commented Jul 17, 2026

Uh oh!

LourensVeen commented Jul 20, 2026

Uh oh!

LourensVeen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maarten-ic commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maarten-ic commented Jul 16, 2026 •

edited

Loading

maarten-ic commented Jul 16, 2026 •

edited

Loading