# Pull Request
## Title
Fixes some test issues for Mac OS environments and enables CI pipelines.
---
## Description
- [x] mypy fixups
- [x] Enable MacOS build pipeline
- [x] ~Enable MacOS devcontainer tests~
- [x] ~Needs `docker` enabled (which isn't currently possible)~
- [x] ~Related: add *basic* devcontainer build test and run for Windows
as well (also isn't currently possible)~
- [x] Fixes MacOS tests
- [x] Address parallel runner issues with output dir cleanup
- [x] Fix ssh tests (also a parallelization issue)
- [x] ~Publish multi arch docker images (amd64 and arm64) (can't see
above)~
Closes#875
---
## Type of Change
- 🛠️ Bug fix
- ✨ New feature
- 🧪 Tests
---
## Testing
Locally tested on MacOS, Windows, and Linux.
---
## Additional Notes (optional)
Leaves some currently disabled stub code for creating devcontainers in
the future on MacOS and Windows hosts. Unfortunately this isn't
currently possible with the Github Action runners.
Also did some cosmetic renaming of the CI pipelines for better
descriptions.
This affects the required tests to pass in the repo settings.
Will adjust that after this is approved.
---
# Pull Request
## Title
Fixups to the devcontainer build for macOS clients.
---
## Description
- Use a more portalable random number generator
- Address some differences in docker.sock privileges.
- Address differences in `stat` arguments.
- Switch to wget mode for conda installation to workaround lack of arm64
apt repo.
- When pulling base images to prime cache, use the appropriate
architecture (for Windows we only support amd64 for now).
- Remove `:latest` from `cache-from` args for `podman` compliance.
- Address some differences in `sed` syntax.
- Fixes#873
---
## Type of Change
- 🛠️ Bug fix
---
## Testing
- local MacBook testing
- CI testing for Linux
---
## Additional Notes
Doesn't currently do builds for arm64 platform in the pipeline.
Can work towards addressing that in the future.
# Pull Request
## Title
Quick fixups to the documentation build.
---
## Description
- Expands one of the ignored warnings.
- Reformats the sphinx conf for black and pylint.
- Removes a warning from that file around redefinition of
`html_theme_path`.
- Adds a dependency requirement tweak for `pyparsing` when installing
under python 3.8 (unrelated other CI bug).
- Adds a workaround to a mypy false positive with in checking `np.e` as
a "deleted variable" (e.g., `e` is used in a try/except block)
---
## Type of Change
- 🛠️ Bug fix
- 📝 Documentation update
---
## Testing
- Ran `make doc` and `make check-doc`.
- Ran `make notebook-exec-test`.
---
## Additional Notes (optional)
This is a quick fix to get the CI pipeline working again.
The more complete fix to remove warning ignores from doc generation,
improve cross reference linking, etc. will be handled later in #869
---
- [x] enable notebook execution in CI pipelines to prevent these
mistakes in the future
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Follow on work to #766.
This enabled both formatters and applies their changes to the repo.
Additionally, since `black` does not make changes to comments nor
docstrings, we also enable `docformatter` to reformat docstrings which
better aligns with `pydocstyle` rules as well.
Without this additional change (and some manual fixups), `pycodestyle`
and `pylint` would still complain about line lengths, for instance.
Finally, we make a minor adjustment to the max line length setting it to
99 (which is also accepted and mentioned in pep8) instead of 88 to avoid
some comment (especially linter overrides) wrapping.
Builds off of #762, #763, and #764.
Prepares rules and configs to enable isort and black formatters and
checks but doesn't enable them yet.
After these are enabled (next PR) we will reformat all files and ignore
that revision in git blame configs.
Then, we can convert configs stored in `setup.cfg` and `.pylintrc` to
the top level `pyproject.toml` and remove the older configs.
This PR builds off of #762 and #763
These are in part
1. followup fixups for #746 (e.g., to allow setuptools-scm to be pulled
in at build time as a build dependency only when conda an pip have
mismatched version issues), and
2. Modernization improvements to allow us to make better use of other
tools (e.g., `black` that only accept `pyproject.toml` files as their
configuration files).
To do so, we move some configs from `setup.py` to `pyproject.toml` for
each module.
However, to retain the ability to rewrite URLs in published README.md
files on PyPi as well as consistent version dependencies across modules
without the need to manually specify version numbers (e.g., using
`setuptools-scm`) we mark a few dependencies as dynamic and leave our
existing logic inside the `setup.py` file.
Finally, we reorganize the `version.py` file to be inside the module and
fix a few previous omissions for `mlos_viz`.
- Introduce a new `make format` target that currently only calls
`licenseheaders` but will call `isort` and `black` in the future
- Introduce new variables to help make dependency tracking easier
- Simplify `pytest` rules and allow running only a single module at a
time.
This PR is a precursor to pyproject.toml related build changes as well.
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
- Mark `mlos_viz` as `typed` for `mypy`
- Bump version
- Mock calls to matplotlib/dabl for testing
- Add plotting of top-N configs
- Improve plots for handling repeat config trials via variance error
bars
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Prior Makefile rule consolidation tried to group dependencies for the
pattern rule into separate lines for readability, but apparently this
isn't allowed.
This minor changes separates those out into a variable we can reference
instead. Tested on a clean build tree locally.
Adds a basic `mlos_viz.plot(exp)` style API for simple visualizations of
`ExperimentData` results relative to the experiment's objectives
(building off of #628 and https://github.com/dabl/dabl/pull/335).
Note: this PR currently omits unit tests for the new module due to the
complexity of testing visualizations. We intend to add this in future
PRs. There is however, a working example of its use here right now:
https://github.com/Microsoft-CISL/sqlite-autotuning/pull/41
---------
Co-authored-by: Sergiy Matusevych <sergiy.matusevych@gmail.com>
Corrects this error:
```
---------- coverage: platform linux, python 3.11.5-final-0 -----------
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml
Required test coverage of 0.92% reached. Total coverage: 93.12%
=========================== short test summary info ============================
```
- fix a quoting issue in an example
- cross link the `sqlite-autotuning` repo
- reuse the content from the main README.md's for the github pages site
- general improvements to the landing page docs
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Adds tests for RemoteEnv `setup`, `run`, `teardown` using SshServices
Fixes some associated bugs:
- convert certain `SFTPError` exceptions to `FileNotFound` in
`SshFileShareService`
This is important for `LocalFileShareEnv` integration especially since
it always calls `download` during calls to `status`, but only handles
`FileNotFound` exceptions when `ignore_missing=True`
- Fixups to #517 for `download`, `upload` to connect via `self._params`
(loaded from the `required_args`) instead of `self.config`
Also included:
- basic configs for SshServices and LocalExec for easy inclusion
Closes#521
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Co-authored-by: Sergiy Matusevych <sergiy.matusevych@gmail.com>
Fixes a bug with mock responses not providing a `json()` method which is
only called during debug logging.
This was somewhat randomly showing up based I think upon the order of
the tests running.
One of them, seemed to be enabling `_LOG.setLevel(logging.DEBUG)` so
that future tests (for that worker) would also have the level set.
Once done, additional paths would be followed inside the `AzureServices`
code that caused it to try and do something like the following
`_LOG.debug(response.json())` which would fail because the mocked
response object didn't provide a `json()` method response.
This fixes that error, but doesn't address the `debug` log ordering.
I'm not certain we should switch all of our unit tests over to running
in DEBUG mode by default since it might cause us to depend on that
behavior.
As a compromise, for now I've enabled it only inside
`.vscode/settings.json` so that when we run `pytest` interactively
(e.g., during dev cycle or while debugging a single unit test), then
those code paths will be followed.
We can also run the following on demand and incorporate it to our
`Makefile` or nightly runs to do it at least some of the time:
```sh
pytest --log-level=DEBUG
```
or
```sh
export PYTEST_EXTRA_OPTIONS=--log-level=DEBUG
make test
```
Extends #340, #346, #349, #352, #359 to Service configs.
See Also: #331
- [x] main schema as a set of subschemas
- [x] connect schema validation in the mlos_bench code
- [x] test-cases
To do this we removed support for configs that were flat lists of service configs: `[ {"class": "service.class"} ]` and turned those into objects that have a single key `"services"` with the list instead. `{ "services": [ ... ] }`
This makes sure that all configs are dicts/objects that we can optionally put `"$schema"` attributes on but also simplifies some of the config loading logic.
* Disable line buffering
* fail pylint checks on unused-import errors
* make sure pycodestyle checks for indentation
* increase code quality requirements
* add some new extensions
* bring pycodestyle checks inline with flake8
* add a prettier rc that instructs it to use our editorconfig
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
* reorg examples/configs to a more consistent layout and start documenting it
* include the config examples in the package
* distribute the config directory
* fixups to exclude tests from the wheel file
* also exclude the test files even from the sdist tars
* test that the configs got included in the wheel
* automatically add the modules included configs to the search path
* fix inconsistent plurals
* add conditional dependency for importlib_resources for python 3.8
* adding mock vm ops services
* make the publicly accessible version a property
* allow llamma tune to always reverse the default config
* handle null categorical values and disallow duplicate values
* add local mock exec service
* more mock services and tunables fixups
* pylint fixups, spelling, more tests for tunable groups loading, and make tunable groups indexable by tunable as well as by name
* merge tunables upstream only
* splitting out tweaks to covariant group names and null value processing for tunables
* add a test for duplicate values being disallowed
* adding some tests for changes to tunable assignment/definition behavior
* mypy cleanup
* rework covariant group merge compatibility check
* return None not 'None'
* fixup score calc
* Create a unified start script for both benchmarking and optimization
* split the Launcher constructor into several private methods
* update the README and doc index with the new script info
* update the Makefile to refer to the new launch script
Adds static type checking via mypy for mlos_bench as well.
Builds on #305, #307
- [x] Add `Protocol`s for different `Service` types so `Environment`s can check that the appropriate `Service` mix-ins have been setup.
- [x] Config Loader
- [x] Local Exec
- [x] Remote Exec
- [x] Remote Fileshare
- [x] VM Ops
- [ ] Future PR: Split VM Ops to VM/Host and OS Ops (for local SSH support)
* force function definitions to have type decorations
* type annotations
* ignore errors on listening to port 81 from 127.0.0.1 so we can run two devcontainer instances at a time
* more type check fixups
* ignore missing matplotlib stubs
* more typing
* spelling
* annotate test functions
* spelling
* types
* spelling
* more typing
* more typing fixups
* more strict
* more typing fixups
* fixups
* more
* spaces
* merge conflict fixups
* more merge conflicts
* fixup
* back that change out
* minor tweaks for unimplemented function
* fixups for py 3.89
* fixups for newer version of mypy
* minor tweaks to dmypy script
* update
---------
Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
* move nginx port to a random port
* make it work for powershell too
* need the .env file to be in a different directory
* fixups for pwsh
* also stop running the docker pull
* whitespace reorg
* fixup
* make the cmd more verbose
* more debugging
* ignore ipv6
Windows (or other platforms) may listen on port 80 and cause conflicts when WSL2 tries to forward ports for both sides so that `localhost` resolution behaves as expected despite the VM technically being a separate host.
When this happens the devcontainer can fail to start due to in ability for nginx docs container to listen on port 80.
To workaround this we simply tell it to listen on port 81 instead. The devcontainer forwards the port to a different one on the host anyways.
Additionally, we document some commands for cleaning up stale container images in the ACR service to avoid excessive storage usage.
* adding configs to be able to use bumpversion for managing versions
* Bump version: 0.0.4 → 0.1.0
* cleanup old file
* don't need to reinstall licenseheaders everytime if we include it in the conda env
* github actions checks out in shallow mode which setuptools-scm doesn't like, but doesn't cause issues, so we ignore the warning
- make test and check targets for mlos_core and mlos_bench independent for faster execution and more clear output for `make test` and `make check` iteration when only hacking on one module locally
- ensures that the full set still runs in the pipeline though
- add support for skipping code coverage locally by setting `SKIP_COVERAGE=true` to run tests faster
- optionally copy the code coverage data into our static html site for publishing (plus nice badges)
- also, rather than racing and publishing the test results and code coverage for all of the different sub jobs, we only run and upload coverage via `pytest-azurepipelines` for the main one we're interested in to make reporting easier
- also includes which job the test came from in the results
- some other general `Makefile` cleanup
Future work:
- [X] Split the single pipeline into two pipelines and make the Windows one optional
(since it's usually the one that takes the longest and we care a little bit less about it now that we have decontainer support)
!1040
Refactoring and documentation improvements.
- Makes more consistent layout matching of `mlos_bench` `tests` and source code directory layouts.
- Places `Services` into separate `mlos_bench.service` module structure.
- Splits these out into separate `local` and `remote` ones, with `azure` being placed under the `remote` ones.
- Makes the `OSEnv` and `VMEnv` `Environments` more genericly `remote` environments.
(they have nothing to do with Azure atm - that's all encapsulated by the `service.remote.azure` used to instantiate them)
- Places `Tunables` into separate `mlos_bench.tunables` module structure.
- Splits multi-class `tunables.py` into multiple files.
- More consistent plural usage (e.g `environment` instead of `environments`).
- Adds `README.md` files to the directory structures.
- Requisite test, doc, etc. fixups to match these reorg.
This is all in preparation to add new SSH remote `Services`.
Additionally, I plan to make `wait_for_vm` more generically `wait_for_host` so that we can handle baremetal machines as well, but that can be a separate PR as it actually changes the API.
Brian Kroth
Sergiy Matusevych
Brian Kroth