Skip to content
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

accumulator couplers need to trigger update on dependency during invalidate() #3288

Open
tclune opened this issue Jan 7, 2025 · 3 comments
Assignees
Labels
0 Diff The changes in this pull request have verified to be zero-diff with the target branch. 🪲 Bug Something isn't working 📈 MAPL3 MAPL 3 Related

Comments

@tclune
Copy link
Collaborator

tclune commented Jan 7, 2025

Currently Accumulator actions accumulate the current value of the input during the invalidate() step. This works when the coupler is directly connected to the user output. But if there is an intervening coupler the "lazy" strategy for updating values interferes with the functioning of Accumulator.

Probably the fix is to add a method to GenericCoupler that controls whether to call update() on dependencies during invalidate(). Who knows, some other coupler might need the same.

@tclune tclune added 🪲 Bug Something isn't working 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. 📈 MAPL3 MAPL 3 Related labels Jan 7, 2025
@tclune
Copy link
Collaborator Author

tclune commented Jan 7, 2025

There might be a sneaky way to do this without changing the general logic: if we can make the dependent coupler part of the AccumulatorAction, then we can call the update() there. Still lots of code to change, but it is at least isolated to the subclass that breaks the pattern.

@darianboggs
Copy link
Contributor

Capturing Thoughts/Questions

  • How to set stale?
  • When to set stale?
  • invalidate on a coupler may initiate update on sources
  • invalidate on a coupler initiates invalidate on consumers
  • update on a coupler initiates update on sources
  • update on a coupler may initiate invalidate on on consumers
  • Must avoid dependency loops
  • Are dependency loops an issue for GridComps in general?
  • Can we insist that a coupler chain be directed with regular GridComps at start and finish?

@tclune
Copy link
Collaborator Author

tclune commented Jan 13, 2025

Unless we also have update() generate invalidate() there is 0 chance of loops. Couplers grow like a tree. (Technically a DAG because of weirdness with vertical).

Worst case scenario - invalidate() always triggers update() and we waste computations. (We very much want to avoid that though.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 Diff The changes in this pull request have verified to be zero-diff with the target branch. 🪲 Bug Something isn't working 📈 MAPL3 MAPL 3 Related
Projects
None yet
Development

No branches or pull requests

2 participants