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

src directory #6

Open
umutacar opened this issue Jan 17, 2022 · 11 comments
Open

src directory #6

umutacar opened this issue Jan 17, 2022 · 11 comments
Assignees

Comments

@umutacar
Copy link
Collaborator

umutacar commented Jan 17, 2022

I would recommend moving the code under a "src" directory
The top level would have things like

  1. virtual env's needed for python (this would be local to the user)
  2. documentation
  3. infrastructure related scripts (e.g., for cloud setup etc if any)

Without a separate an "src" directory, it is difficult to do a number of things, for example, visually separating varioun configuration/infra matters from actual code. It is also difficult to search the source code because for example the virtual env code will get mixed in, which makes search (e.g., grep -r) effectively unusable (BAD).

@xumingkuan
Copy link
Collaborator

I think moving the code under a "src" directory is already done by @zikun-li in #2. Actually, all .cpp code outside this directory are deprecated after this PR and can be safely deleted.

However, the Python code is not refactored in the PR. I think one way to do the refactoring is to put them into src/python. WDYT?

@umutacar
Copy link
Collaborator Author

umutacar commented Jan 20, 2022

We are looking over the code with Mingkuan now. Couple of observations

  • There is code duplication, e.g., context/context.h and includes/context.h
    We probably want just one of these
  • We still do not have all the code under src. we should move
  • We seem to be using "tests" to mean benchmarks and a few are for correctness
    We should use "test" to mean correctess and "benchmark/experiment" to mean run-time measurement etc.
  • We can set up a separate "test suite" that checks that the compiler is correct. At each pull request, we should set up github to run these tests. To do this, we create a .github/workflows directory and spec the tests to run
  • We also could not run the tests because of path issues

@zikun-li
Copy link
Collaborator

Got it. For 1 and 2, I'll delete the deprecated files, and organize all the .h and .cpp files into the src/ directory. For 3, 4 and 5, we plan to separate all the tests files into two directories, src/test for correctness and src/benchmark for benchmark. I'll dicuss with Mingkuan to figure out how to construct test suite and benchmarks.

@xumingkuan
Copy link
Collaborator

xumingkuan commented Jan 22, 2022

Except for src/test and src/benchmark (or src/experiment), maybe we also want to put python into the src/ directory (and also utils should be src/python/utils, verifier should be src/python/verifier)?

@zikun-li
Copy link
Collaborator

That's right, I gonna move these files.

@zikun-li
Copy link
Collaborator

I see there is a requirement.txt file under the python directory. There should be no python directory after moving all python files to src/python/, maybe we should put the requirement.txt under the top directory?

@xumingkuan
Copy link
Collaborator

I see there is a requirement.txt file under the python directory. There should be no python directory after moving all python files to src/python/, maybe we should put the requirement.txt under the top directory?

Oh maybe we should not move that. src/ directory is for the source code, and requirement.txt is for configuration. Putting it under python/ shows that it's for Python configuration.

@zikun-li
Copy link
Collaborator

@xumingkuan there is a verify_equivalences.py file under python/ directory, is it a test file?

@xumingkuan
Copy link
Collaborator

No, verify_equivalences.py should be count as source code and it actually provides an API for C++ code to invoke as system("verify_equivalences.py ...").

@zikun-li
Copy link
Collaborator

Then I'll put it into src/python/verifier/

@xumingkuan
Copy link
Collaborator

Ok sounds good.

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

No branches or pull requests

3 participants