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

Branching #21

Closed
wlandau opened this issue Apr 8, 2020 · 18 comments
Closed

Branching #21

wlandau opened this issue Apr 8, 2020 · 18 comments

Comments

@wlandau
Copy link
Collaborator

wlandau commented Apr 8, 2020

In targets, all branching will be dynamic branching.

Let's tackle the infrastructure before we get too far ahead. One idea: we could use target iterators/generators. Maybe the abstract factory pattern?

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 9, 2020

Every target should know how to get a branch of itself. Maybe with methods get_branch() and find_branch() and read_branch().

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 11, 2020

No matter what we do, let's handle this at the target level before we even implement the DSL, the graph, or the priority queue.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 11, 2020

Got a good start with this. Non-branching targets can get and find their own branches and count the number of branches.

@wlandau wlandau changed the title Dynamic branching Branching Apr 11, 2020
@wlandau
Copy link
Collaborator Author

wlandau commented Apr 11, 2020

Now we need to implement a branching target class that inherits from the regular non-branching class. The differences in methods will mainly be in how they store and retrieve data. A big challenge will be keeping track of branches and making sure to ignore or delete old stale branches from previous builds.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 12, 2020

Next actions:

  • Create a targets_brancher class with:
    • Field index: numeric index of the current branch
    • Field names: names of the branches
    • Method get_names()
    • Method get_index()
    • Method get_name(index)
    • Method count_names()
    • Method set_names()
    • Method set_index()
    • Method inc_index()
    • Method reset_index()
    • Method validate()
  • Create a targets_branching class.
    • Should have a brancher field.
    • Should implement different versions of the methods of targets_target. read_value() and friends should still get the whole value. read_branch() etc. should get individual branches.
  • Move the common methods across branching and non-branching targets to a common parent abstract class. Shift non-branching targets to a new targets_nonbranching class. targets_nonbranching and targets_branching should both inherit from it.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 12, 2020

The general idea of #21 (comment) is, as always, to chip away at the easiest cleanest fiddly bits of this large problem until the pieces all fall into place.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 14, 2020

I no longer think this is a good idea. We should have one target class. Some targets have branching, some are branches. That's pretty easy to tackle in settings, and creating a new class for that is overly complicated.

Since branching is dynamic, it is really a job for the scheduler, which has the priority queue and graph. So that should be the next action item.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 19, 2020

Do we need #22 first?

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 20, 2020

Whenever a target completes, we need to update which branches are available. We can create a new brancher class to handle that.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 21, 2020

Changed my mind: inheritance is perfect for this. A target can be either a pattern or a singleton. Patterns are map or cross, singletons are unique or branches.

@wlandau
Copy link
Collaborator Author

wlandau commented Apr 22, 2020

For starters, let's separate out the fields and methods of the existing target class into the target, singleton, and unique classes.

This was referenced Apr 26, 2020
Closed
@wlandau
Copy link
Collaborator Author

wlandau commented Apr 26, 2020

Notes to self:

  • Use the presence of the childlist to infer whether a stem's buds are in the graph yet.
  • pattern$ensure_children() should ensure the children of all the branch-related dependencies and then insert its own children.
  • Only create a bundle if a downstream target will use it. Requires checking the graph in the step above.

This was referenced Apr 27, 2020
@wlandau
Copy link
Collaborator Author

wlandau commented May 1, 2020

I am so close now, I just need to debug with simpler plans.

@wlandau
Copy link
Collaborator Author

wlandau commented May 3, 2020

Almost done. Just need to amp up the testing.

@wlandau
Copy link
Collaborator Author

wlandau commented May 3, 2020

  • The current plan_map() tests and plan_cross() tests need actual checks on the values and what is in memory.
  • Amp up to larger plans with more complicated dependency structures.
  • Need to test that values get loaded into memory and do not need to be re-read from storage.

@wlandau
Copy link
Collaborator Author

wlandau commented May 3, 2020

Done!

@wlandau wlandau closed this as completed May 3, 2020
@wlandau
Copy link
Collaborator Author

wlandau commented May 4, 2020

Nope, had issues. Not quite figured out.

  • Fix tests.
  • Fix coverage.
  • Think about what happens when a stem is skipped but the buds need to be loaded from memory. How do we make sure the stem gets unloaded again? Simple: we push the stem to the downstream priority queue with rank 0 if it is not already there. But now we need to take the scheduler as an argument of load_deps().
  • Test the above.
  • Explain algorithm in the spec.
  • Make sure bundles are removed from the spec.

@wlandau wlandau reopened this May 4, 2020
@wlandau
Copy link
Collaborator Author

wlandau commented May 4, 2020

Fixed for real this time. The solution is clean.

@wlandau wlandau closed this as completed May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant