search

StanfordAHA / CGRAFlow

Integration test for entire CGRA flow
BSD 3-Clause "New" or "Revised" License
12 stars 2 forks source link

Nbdev2 #52

Closed steveri closed 6 years ago

steveri commented 6 years ago

This is phase 1 of the master plan, see email for details. These changes should suffice to make all our existing tests (8x8 pointwise, conv12, conv21, conv31, convbw) pass in travis using SMT-PNR along with the latest version of the CGRA generator (nbdev2).

rdaly525 commented 6 years ago

Because there are changes to the flow, the following should happen roughly at the same time:

-Do one final commit in this CGRAFlow branch changing the names of the P&R/CGRA branches back to 'master'. -Merge this pull request. -Merge P&R and CGRA branches into their own master. -Verify that master for the flow did not break.

steveri commented 6 years ago

Re: "the following should happen roughly at the same time"

Actually I was thinking we could preserve a 'never-broken' master by doing the following:

  1. check in this master, which is working, and which points to 'nbdev2' and 'io_tile' branches.

  2. At their leisure (but presumably soon) Steve merges CGRAGenerator/nbdev2 to CGRAGenerator/master and Caleb or Makai merges SMT/io_tiles to SMT/master such that now

  1. Again at their leisure (but presumably soon), CGRAFlow is redirected to point at CGRAGenerator/master and SMT/master.

To that end, I will go ahead and merge this pull to get the ball rolling...

SR

leonardt commented 6 years ago

I would vote for rewording at their leisure to ASAP. I believe we already saw a potential issue related to stale master branches (issue with taeyoung's base branch). In terms of methodology, I think it's best practice to always keep master fresh. This way, a team member can branch off master with confidence that it's up to date, and they don't have to hunt down the right branch to base off of.

leonardt commented 6 years ago

I would also vote that we don't merge into CGRAFlow master with the travis script pointing to non-master branches (again to promote the idea that individual repositories are master is fresh)

makaimann commented 6 years ago

I agree that branches should be merged ASAP. PNR io_tiles branch is ready to be merged. In keeping with other good practices, I'm having @cdonovick review and commit my changes. I think he's busy today, but I anticipate that will be merged by EOD Tuesday. Regarding CGRAFlow master pointing to non-master branches, I agree it's probably better not to. I don't think it's worth reverting though.

steveri commented 6 years ago

Sorry, I guess I jumped the gun. FWIW at this point I merged CGRAGenerator/nbdev2 and CGRAGenerator/master, and CGRAFlow is currently pointing to CGRAGenerator/master and passing (build 1209). Looks like SMT branches will be sorted out soon, and maybe we can move forward from there.

In keeping with the will of the crowd, I will try and remember to order it differently next time around... :)

steveri commented 6 years ago

There's a third option, I guess, that may or may not make everyone happy. It preserves the never-break rule for master and also the (new?) master-only-points-to-master-sub-repositories rule. Next time we could do the following, unless people decide it's too complicated and/or prefer a different path:

  1. Build a CGRAFlow/staging branch that points to the new sub-repository branches
  2. merge all sub-repository branches to their respective master branches
  3. Switch CGRAFlow/staging to point to master branches only
  4. Merge CGRAFlow/staging to become the new CGRAFlow/master
steveri commented 6 years ago

PS On the previous comment, in case it wasn't obvious, in our current situation it would have been the case that 'staging' == 'nbdev2'. Which I guess is what Ross was proposing all along, wasn't it :)

leonardt commented 6 years ago

That seems like a reasonable path. Should be possible to write a script that uses the GitHub API to perform those sequence of merges in order once the original (step 1) staging branch is green. I'd be willing to take that on once I get some time

cdonovick commented 6 years ago

P&R is merged.

steveri commented 6 years ago

Okay I changed CGRAFlow/master to point to ONLY MASTER BRANCHES, maybe now the world will be right again :)

steveri commented 6 years ago

master build passed, maybe we're finally done with this thread?

https://travis-ci.org/StanfordAHA/CGRAFlow/builds/338615543