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

Port Over Simulator and Compiler from pifo-tree-artifact #57

Merged
merged 22 commits into from
Sep 1, 2024

Conversation

polybeandip
Copy link
Contributor

@polybeandip polybeandip commented Aug 6, 2024

This PR makes the following changes

  • fill in ocamlformat
  • remove bin/
  • alter WFQ to weights to be float instead of int (necessary to implement WFQ_Ternary in schedulers-in-ocaml)
  • alter file structure to have two libraries, Frontend and Simulate
  • remove NWC policies and EDF, SJN, and SRTF in policy.ml; constructing controls for them is for later
  • port over simulator and compiler from schedulers-in-ocaml
  • implement Control.of_policy (most of the work for this PR)
  • setup T2T compilation tests in tests/compilation: i.e. check the .csvs match
  • add two work conserving programs: ternary strict and wfq

To generate graphs via our simulator, cd into dsl/ and run

dune clean; dune build; dune test; python ../graphs/plot.py

This populates graphs/

@polybeandip polybeandip changed the title Port Simulator from pifo-tree-artifact Port Over Simulator from pifo-tree-artifact Aug 6, 2024
@polybeandip
Copy link
Contributor Author

Yahoo, all graphs match now!

@polybeandip polybeandip marked this pull request as ready for review August 7, 2024 21:17
@polybeandip
Copy link
Contributor Author

polybeandip commented Aug 7, 2024

I'm sorry for the size of the diff.

While the additions are inflated by copying code from schedulers-in-ocaml, this still should have been multiple PRs; I got a bit carried away with making changes.

@polybeandip polybeandip changed the title Port Over Simulator from pifo-tree-artifact Port Over Simulator and Compiler from pifo-tree-artifact Aug 7, 2024
@anshumanmohan
Copy link
Contributor

Super duper! Before I do a real review, a quick question: in copying over schedulers-in-ocaml are you taking us to a monorepo model? So both the pieces of code live here in this repo and we eventually delete the other repo? An alternate to deletion is to use a git submodule to house what is currently schedulers-in-ocaml.

The broad alternate to a monorepo is that we actually keep the repos separate, and that we request the user to download the two as sibling directories. Then we write some interstitial code that reifies the connection between the two repos. By and large I think @sampsyo and the lab are a little iffy about this second style

@polybeandip
Copy link
Contributor Author

polybeandip commented Aug 9, 2024

I copied schedulers-in-ocaml code because it seemed like a quick way to connect our DSL to a T2T compiler and simulator: it wasn't a very well thought out decision 😬

are you taking us to a monorepo model?

Long term, I'm conflicted on what the best choice is.

If we ultimately decide to use this T2T compiler for converting between topologies (i.e. following the blue path in #38), then moving to a monorepo model makes sense. Maybe just the simulator can be separated out? However, if we choose not to use this compiler, we'd likely rewrite it (either from and to DSL programs or between Calyx eDSL programs). In that case, maintaining two separate repositories or a git submodule seems best, as our simulator and OCaml compiler would be auxiliary utilities rather than central to our workflow.

@anshumanmohan
Copy link
Contributor

Agreed; not clear what would be best. But then again it doesn't seem like it would be too too hard to reverse any of these decisions as long as we promise to avoid any large changes to theschedulers-in-ocaml code, either here or there. With that promise, let's just have you go ahead. I don't want to bikeshed this. Review coming up!

@sampsyo
Copy link

sampsyo commented Aug 9, 2024

"Let's just do it and worry about the right final decision later" seems about right to me!

Copy link
Contributor

@anshumanmohan anshumanmohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Some little things in here for you but otherwise you're clear to merge

dsl/dsl.opam Outdated Show resolved Hide resolved
dsl/dune-project Outdated Show resolved Hide resolved
dsl/frontend/frontend.ml Show resolved Hide resolved
dsl/frontend/policy.ml Outdated Show resolved Hide resolved
dsl/frontend/policy.mli Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/tests/compilation.ml Outdated Show resolved Hide resolved
Copy link
Contributor

@anshumanmohan anshumanmohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrighty thanks for flagging the missed file. It is indeed a bit dense so I've requested a few comments and type annotations! Plus I think I spotted once chance for OCaml-level tidying

dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
dsl/simulation/control.ml Outdated Show resolved Hide resolved
- rename:
	- rank_less_path -> rankless_path
	- z_in -> z_in_of_policy
	- z_out -> z_out_of_policy
	- route_pkt_opt -> route_pkt
- remove pkt arg from route_pkt_aux
- convert State to association list
- Document Control helpers
- Move around comments in path.ml and pieotree.ml
@polybeandip polybeandip merged commit 4421006 into main Sep 1, 2024
3 checks passed
@polybeandip polybeandip deleted the dsl+artifact branch September 1, 2024 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants