-
Notifications
You must be signed in to change notification settings - Fork 11
Description
From PR #444: "Create multiple PODs where resource limits for a single POD are exceeded"
1. Copy operations incorrectly consume private statement slots
File: src/frontend/multi_pod/solver.rs:438
Original comment by ed255:
I don't understand this part. You're considering a copy as taking a slot of the private statements.
But my understanding is that this copy happens when statement s depends on statement d. Statement s is proved in p2 and statement d is proved in p1. We must expose statement d as public in p1, and have p1 as input to p2. After this, d is available in p2, but it doesn't consume a private statement slot (it's available in the input pods public statements area).
p1: priv: d pub: d <- Copy d p2: input: p2 d priv: s <- d
2. Simplify contains statement handling
File: src/frontend/multi_pod/solver.rs:537
Original comment by ed255:
I wonder if we could get rid of this logic by using the list of statement that the MainPodBuilder generates (with the unlimited params), where the contains statements have already been created, and counting them as regular dependencies to the statements that use them.
3. Remove unnecessary constraint and optimization
File: src/frontend/multi_pod/solver.rs:626
Original comment by ed255:
The current implementation finds a solution by iterating over a growing number of target_pods. I believe the first value for
target_podsis a lower bound. This means that if a solution is found, then all pods are used; which makes this constraint unnecessary?I also realized that the minimization of pods_used doesn't make sense now, because if a solution is found, all the pods will be used. But the optimizer requires a max or min target.
4. Simplify caching design
File: src/frontend/multi_pod/mod.rs:188
Original comment by ed255:
Is there a reason to keep this in an Option? There's a method called
invalidate_cachethat doesn't clear this. Is that intended?BTW I think the design of this would be simpler if
solveandproveconsumeself; this way you can forget about cache invalidation.
5. Remove unnecessary Copy statement
File: src/frontend/multi_pod/mod.rs:519
Original comment by ed255:
This Copy statement is not needed. When a pod A contains an input pod B, all the public statements of pod B are available in the namespace of pod A.