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

Refactoring our builds and tests #707

Open
3 tasks
elegios opened this issue Mar 14, 2023 · 11 comments
Open
3 tasks

Refactoring our builds and tests #707

elegios opened this issue Mar 14, 2023 · 11 comments

Comments

@elegios
Copy link
Contributor

elegios commented Mar 14, 2023

Our build and test system is getting a bit unwieldy, and it currently requires a large number of files at the top-level of the repo. We'd like to address both of these points, and this issue is intended to track that effort.

Tasks

  • Collect information on all "strange" tests; those that need extra dependencies, different flags, or some other way to run them.
    • For comparison, I think that all mc files should have the following tests by default:
      • Compile with mi compile --test.
      • Interpret with mi eval --test (and no arguments to the program itself).
      • Run the compiled version, also with no arguments.
    • For the strange tests I imagine the following information is relevant:
      • Dependencies required and/or how to detect if they're installed, if relevant.
      • Normal files, mc-files that are tested as normal but only if the dependencies are available.
      • Excluded files, mc-files that should not be tested as normal, even if dependencies are available.
      • New tests, how to run other tests, e.g., mc-files that require other flags or arguments, or things that are run in entirely different ways.
      • Other exceptions, if some file should skip interpretation or running, e.g., because it's too slow for interpretation or because it requires some feature present in the compiler but not the interpreter.
  • Decide on the interface for invoking the build system. I have an initial sketch below, slightly modified from what we have right now.
  • Decide on how we should implement the build system, i.e., how we keep it understandable and maintainable.

Targets/interface

  • Testing
    • test-quick, a small set of tests that use no optional dependencies, as a sanity check.
    • test, run all tests for which dependencies are installed, i.e., it is "smart". This should print what was chosen and what wasn't.
    • test-all, like test, but unconditionally run all tests, i.e., it fails if there's a dependency missing.
  • Building
    • build/mi, the default target via full bootstrapping. Should maybe overwrite an already built compiler, so it doesn't need to be combined with make clean all the time?
    • Bootstrap via installed mi.
    • boot, for the interpreter and the library.
  • Installing
    • install, install (already built) mi executable and boot library.
    • install-boot, install the (already built) boot library.
  • Other
    • watch, to watch for changes via entr and run tup
    • clean, remove all files in the repository that are ignored (i.e., via .gitignore), which should include all build artifacts.

Questions/problems:

  • Targets for each special kind of test? Maybe the normal test could (optionally) be told exactly which sets of tests to run, and test-quick and test-all are just special cases?
  • It's sometimes useful to use different versions of mi for the tests (installed, full bootstrapped, or bootstrapped via installed).
  • Is it interesting to have a version of watch that does not use tup? It would presently not handle dependencies, i.e., everything would be re-run all the time, which is probably not all that useful?
  • Sometimes it's useful to watch but only re-run a subset of the tests.

Collected test information

  • Accelerate (test/examples/accelerate/**/*.mc)
    • Dependencies: CUDA (which nvcc) and Futhark (which futhark). Also hardware requirements, so should perhaps not even be included unconditionally in test-all.
    • Excluded files: It’s not wrong to test the files as usual, but it’s unnecessary since the compilation and execution are covered when compiling in debug mode.
    • New tests: mi compile --accelerate and mi compile --debug-accelerate, followed by running as usual.
    • Other exceptions: Currently, acceleration is only supported in compiled mode, so these tests should not run in interpreted mode.
  • Sundials (stdlib/sundials/**/*.mc)
    • Dependencies: ocamlfind query sundialsml
    • Other exceptions: The externals aren't available in interpreted mode, so they should fail there. Additionally, sundials is presently not really installable, so maybe it should be fully turned off?
  • Tuning (test/examples/tuning/**/*.mc)
    • New tests: mi tune --test --disable-optimizations --compile --disable-exit-early --enable-cleanup, followed by running as usual.
    • Other exceptions: Don't run these in interpreted mode.
  • Constraints (stdlib/cp/solve.mc)
    • Dependencies: which minizinc
  • Java backend (stdlib/jvm/compile.mc)
    • Dependencies Java (which java and which javac), should perhaps check version (>= 11?).
@larshum
Copy link
Contributor

larshum commented Mar 14, 2023

For accelerate, there are specific test files (test/examples/accelerate/*.mc) that use acceleration, for which we try compiling and executing both with acceleration enabled and in debug mode (where we verify that the well-formedness checks pass). Relevant information for these tests:

  • Dependencies: CUDA (which nvcc) and Futhark (which futhark)
  • Excluded files: It’s not wrong to test the files as usual, but it’s unnecessary since the compilation and execution are covered when compiling in debug mode.
  • New tests: mi compile --accelerate and mi compile --debug-accelerate
  • Other exceptions: Currently, acceleration is only supported in compiled mode, so these tests should not run in interpreted mode.

Note that these tests are currently not included in make test-all. They have hardware requirements (a CUDA-compatible GPU), so I don’t think they should be included there for now.

@johnwikman
Copy link
Contributor

Would this new Makefile setup be a wrapper for tup or a reworking of what we have now? I think the tup system looks nice and it would tie in nicely with the "quick" tests. Some additional comments:

  • Using Makefile patterns, you could parse additional target aspects to specify non-standard aspects that you also want to test. Such as make test/accelerate (quick tests + accelerate), make test/js (quick tests + javascript), or make test/accelerate/sundials (quick tests + accelerate and sundials). For example, I if I changed the underlying CUDA installation and want to rerun the accelerate tests only I could do it as make test/accelerate -B.
  • When you say "mi and boot library", do you refer to the executables? The Boot library is a dependency and must be installed, but I do not think that it makes sense to install the boot binary by default since that is not something you would use outside of the miking bootstrapping process.

@elegios
Copy link
Contributor Author

elegios commented Mar 15, 2023

Note that these tests are currently not included in make test-all. They have hardware requirements (a CUDA-compatible GPU), so I don’t think they should be included there for now.

This is an interesting and somewhat inconvenient point. I guess the ideal case is that if you run test-all without the correct hardware it should give a warning but not run those tests, and if you have the correct hardware it should run the tests whether you have the dependencies or not?

Also, @larshum, I'm guessing the files compiled with --accelerate and --debug-accelerate should also be run (with no arguments?), not just compiled?

Would this new Makefile setup be a wrapper for tup or a reworking of what we have now?

I don't think we should move to tup as the main/only build approach just yet, partly because of the FUSE difficulties on Mac, and partly because there's an issue that needs to be fixed before our setup works properly (I'm using a locally patched version until then).

  • Using Makefile patterns, you could parse additional target aspects to specify non-standard aspects that you also want to test. Such as make test/accelerate (quick tests + accelerate), make test/js (quick tests + javascript), or make test/accelerate/sundials (quick tests + accelerate and sundials). For example, I if I changed the underlying CUDA installation and want to rerun the accelerate tests only I could do it as make test/accelerate -B.

I did not know you could do quite that level of fanciness in make targets. It would certainly be convenient with a quick way of specifying different subsets. It's worth noting though that we're not really using make for it's intended purpose in most of this design (essentially no dependency tracking, just a collection of tasks to run), so it's not a given that we want to keep using make.

  • When you say "mi and boot library", do you refer to the executables? The Boot library is a dependency and must be installed, but I do not think that it makes sense to install the boot binary by default since that is not something you would use outside of the miking bootstrapping process.

I meant the mi executable and the boot library, but not the boot executable. I've updated the original issue to clarify.

@larshum
Copy link
Contributor

larshum commented Mar 15, 2023

Also, @larshum, I'm guessing the files compiled with --accelerate and --debug-accelerate should also be run (with no arguments?), not just compiled?

Yes, that's the idea — both compiling and executing.

@br4sco
Copy link
Contributor

br4sco commented Mar 15, 2023

It is still not possible to build sundialsml with the opam switch we use with Miking and I'm not able to build it from source on Fedora 37.

If you want to check if it is installed I suppose you can do a ocamlfind query sundialsml. For the ipopt interface you can check for the required opam package with ocamlfind query ipoptml.

As you probably already know, the tests for these two are currently defined in test-sundials.mk and test-ipopt.mk.

@lingmar
Copy link
Contributor

lingmar commented Mar 28, 2023

  • The files in test/examples/tuning are run with mi tune subcommand, as defined in test-tune.mk.
  • There are some tests in stdlib/cp/solve.mc that require minizinc (which minizinc) to be installed. Right now, these tests do not run if minizinc is not available.

@elegios
Copy link
Contributor Author

elegios commented Apr 4, 2023

It is still not possible to build sundialsml with the opam switch we use with Miking and I'm not able to build it from source on Fedora 37.

If you want to check if it is installed I suppose you can do a ocamlfind query sundialsml. For the ipopt interface you can check for the required opam package with ocamlfind query ipoptml.

@br4sco Since these would be using externals I assume we expect them to fail when interpreted? The current compile tests seem to disable both optimizations and prune-utests, is that important here?

  • The files in test/examples/tuning are run with mi tune subcommand, as defined in test-tune.mk.

@lingmar I'm guessing that these tests should work in interpreted mode, but that they might be too slow for it? Maybe only some of them?
Also, should they be compiled and run as normal as well, or just via tune?

@lingmar
Copy link
Contributor

lingmar commented Apr 27, 2023

@lingmar I'm guessing that these tests should work in interpreted mode, but that they might be too slow for it? Maybe only some of them? Also, should they be compiled and run as normal as well, or just via tune?

They don't have to be compiled and run as normal, via tune is enough. Tuning is not supported in interpreted mode so that is not needed either.

@br4sco
Copy link
Contributor

br4sco commented May 15, 2023

@br4sco Since these would be using externals I assume we expect them to fail when interpreted? The current compile tests seem to disable both optimizations and prune-utests, is that important here?

Sorry for the late reply. It is not important. You cannot currently do a test-all-prune-utest on the non-multicore switch unless you apply this patch.

diff --git a/test-files.mk b/test-files.mk
index 82b4601d..71c4f13a 100644
--- a/test-files.mk
+++ b/test-files.mk
@@ -60,6 +60,7 @@ compile_files_exclude += test/mlang/also_includes_lib.mc
 compile_files_exclude += test/mlang/mlang.mc
 compile_files_exclude += test/mlang/nestedpatterns.mc
 compile_files_exclude += test/mlang/catchall.mc
+compile_files_exclude += stdlib/multicore/thread.mc
 
 
 # Programs that we currently cannot interpret/test. These are programs written

I don't remember exactly why, but some dependency isn't pruned.

@asta12
Copy link
Contributor

asta12 commented May 31, 2023

The JVM backend stdlib/jvm/compile.mc :
Dependencies:
java and javac. I have been using version 17, but I think the lowest you can go is 11. Java 8 which is the most common one does not work!
To see if they exist use which java and which javac.
You can check the version with java --version and javac --version. This works for HopSpot, other JVMs might have other flags for checking the version.

@asta12
Copy link
Contributor

asta12 commented Jun 5, 2023

One more thing, I need some jar files to compile the java program used in the backend. Right now I fetch them from mvn repositories with curl in the backend. I do not know if it is better if they were fetched by some other script or something which prepares testing/running of the backend.

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

6 participants