Revert markdown lint check package vers pinning #209
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Create initial github actions pipeline for building on windows * add the build script referenced in the github actions ci pipeline * comments * commit something that should fail the uncrustify check just to make sure that the pipelines fail * Revert "commit something that should fail the uncrustify check just to make sure that the pipelines fail" This reverts commit 3fee079. Manually verified that this does indeed fail the build pipeline. * tell editors to use 2 spaces instead of 4 by default for yaml files
* cleanup spacing/comments from previous copy/paste/edit a tad * initial attempt at enabling various python checks in the ci pipelines note: here we run them on linux instead of windows for easier scripting, though (hopefully) the results are more or less that same in either spot (excepting for lf/crlf line ending differences which we should eventually fix separately) * various python action pipeline changes - try to run some of the checks on both windows and linux - try to support a couple of different versions of python? - run the lint and unit tests from the same workspace (saves on some runner startup cost) - try to use the "approved" python setup task from github (else the PATH env doesn't seem to work out right) * test failing the python licenseheaders check * attempting to fix running python checks on linux * Revert "test failing the python licenseheaders check" This reverts commit 9ca6b96. Manually confirmed that this does indeed fail the build as expected. * separate and document windows/linux commands for running python checks * add a bash equivalent for the grpc generation cmd while we're at it * disable python 3.8 checks for now It's failing pylint checks with some "useless statement" warnings
…s (After Rebase) There was a bug in partitioning the input space in the homogeneous random forest. As a result the individual decision trees would return a Prediction object even if the features were outside of their input space.
In preparing the search space for the Gaussian process we erroneously ordered the dimensions in the order they were specified in the OptimizationProblem, but the bayes_opt reorders them alphabetically.
Some selections taken from !410291 ```sh git checkout -b bpkroth/some-linux-cpp-fixups master git format-patch --stdout master..bpkroth/oss-linux-build-work -- *.cpp *.h *.inl *.vcxproj | git am ``` - un-inline some MlosPlatform.Std.inl functions - comments referencing MlosPlatform.Std.ink - tweaks to allow SmartCache to build on Linux - partial (incomplete) work on enabling additional C++ projects on Linux - comment fixup - fixups to allow some more compilation on Linux - use the macro we defined - file can't include itself? - more build fixups for clang - more fixups to allow Mlos.Unit to link - precompiled header fixup for building on Windows Related work items: #806033
Version2 of PredictionClass. _From Adam's email:_ **Problems with current implementation** First, returning a Prediction object for each invocation of bayesian_optimizer.predict() means that to draw one heatmap we need to return 10,000 objects (100x100 pixels). This eliminates hope for memory locality, puts a strain on allocators, requires us to reassemble a numpy array for the benefit of matplotlib, etc. Returning a single data frame would be much preferred. (Pandas data frame also opens door for using Arrow down the road.) Secondly, most of the time we don’t care about most of the values contained in the Prediction object. Each utility function cares only about one or two, the tomograph cares only about the mean and nothing else right now. Computing all the extra values in the Prediction class is a waste of CPU and memory. This PR is a restart of the abandoned: https://msdata.visualstudio.com/Database%20Systems/_git/MLOS/pullrequest/413447 Related work items: #743670, #863921
* Fixed Tomograph to work again with the new Prediction class * added a notebook to demonstrate active learning on smart cache in python * cleaned up MLOS SDK and MLOS Infra for Python * Fixed a bunch of bugs * Made sure that BayesianOptimizerProxy once again conforms to OptimizerInterface
Andreas pointed out that to the user of the BayesianOptimizer the interaction between these parameters is confusing and can lead to subtle bugs:
{maximize, minimize} x {upper_confidence_bound, lower_confidence_bound}.
As one would often use upper_confidence_bound for maximization and lower_confidence_bound for minimization .
However, these parameters are set in different places (OptimizationProblem vs. OptimizerConfig) and entirely non-intuitive. So this PR renames them to upper_confidence_bound_on_improvement and lower_confidence_bound_on_improvement and changes the math accordingly. This way, when the user switches from maximizing, to minimizing, she/he/they doesn't/don't have to change the optimizer config. This rename is in-line with the names of other utility functions: Probability of Improvement, Expected Improvement.
Pandas spits out tens of thousands lines of warnings during Unit Tests runs. This PR cleans it up.
Various fixes in C++ projects to enable Linux build. Do not use string_view in Mlos.UnitTest project. Change include order (include template specialization before using them).
Using sample variance instead of mean variance to compute the confidence intervals makes them not shrink as we add more observations. While this measure can still be useful down the road to detect noisy-TV type of problems, using mean variance makes more sense for the problems we are solving now.
This PR ensures default parameters are copied and not accidentally modified by the user. I'm also introducing a long-haul test for BO, which was the original motivation. Happy to split that off into a different PR. I wasn't sure where to put the DefaultConfigMeta helper, lmk if there's a better place. There's two reasons this is tricky: DEFAULT is used on the class, but properties are instance-level, so we need a meta-class. Also, because of the way the dictionaries are instantiated, we can't make copies of nested points easily, so we need to use deepcopy. This solution still has the issue that we're using mutable objects as default parameters. If a user would modify ``_DEFAULT`` for some reason, the default instantiations would change.
* init website * remove other themes * add basic config * remove non-existant submodules * fix site building script * add empty index, update base url * add generated website to gitignore * fix booksection config * remove unused folder * add link to python api to hugo page * fix link to python api * add docs on python only installation * add generated website to gitignore * remove submodule with book theme * remove generated assets * add code to download book theme to build_site.sh * add if for book checkout * fixup readme a bit * update links in readme, clean up some empty files * remove empty gitmodules * remove generated files * add manual menu, don't lowercase files * try to make links work in hugo and on github * fix up links for hugo * easier way to get python docs into menu * don't have license in hugo menu * fix regex * fix python api doc link again * check out specific tag of book theme (v8) * clarify formulation. * add comment to explain sed commands * update menu, fix link to bayesian optimization * add sed line to change paths for BayesianOptimization notebook
* add sphinx build to gitignore * add apidoc templates * mostly works * slightly cleaner * clean up sphinx apidoc generation * update index * fix source links in python api docs * restructure docs with one more menu level * change link name to main website * add python api index document
* add template for notebook conversion, add links to notebook download * don't keep before file in sed * make download more prominent in template * fix string replace comment
python documentation fixups from microsoft#12 and !422716
Adding goodness of fit measurements for the training (and some validation) datasets, along with a way to query these statistics via gRPC.
Implement support for Posix Named Shared Memory and Named Semaphores. SmartSharedChannel builds on Ubuntu16.04
Added r_squared
In preparation for the demo I'm running the code over and over and I've accumulated a lot of small improvements. The fixes include: * Point.to_pandas() -> Point.to_dataframe() rename * New: Point.from_dataframe() * New: RegressionModelFitState.to_dataframe() * SmartCacheWorkloadLauncher takes duration_s as parameter * RandomSearchOptimzier utilizes Hypergrid.random_dataframe() rather than rolling its own * Tracer collects data about dataframes passed as function arguments (hence all of the args->kwargs changes) * Unskipped a couple of long haul tests * Added some convergence state tests to bayesian optimzier tests * Added: assert suggested_config in self.optimization_problem.parameter_space to BayesianOptimizer * Setting attributes on Point works as expected now. * Cleaned up a broken Unit Test
# What's changing? Second step in simplifying the Hypergrids. In this PR I eliminated the CompositeHypergrid entirely, and all of its functionality is now present in the SimpleHypergrid. All unit tests are passing and I tested all the notebooks too. The main change occurs in Hypergrids.py - most other changes are related to testing code and suitable renames. The one exception is the HypergridJsonEncoderDecoder.py where there is a functional change. This change is backwards compatible, so old code (in C#) will continue working. # Effects on client code None - CompositeHypergrid was almost entirely internal and never instantiated directly by the user. # Reasons for the change 1. It's much simpler to write the recursive functions required for the Hypergrids when there is only one class, rather than two classes. 2. Greg is implementing Hypergrids in C# and it makes no sense to duplicate something complex, if we can duplicate something simple. # Next Steps There will be some renames: GuestSubgrid -> JoinedSubgrid, etc. I will coordinate them tightly to make sure people's branches continue to work well.
* Tweaks to Python Test CI pipelines - Increase the timeouts on the pytest runs. There have been some timeouts recently, due in part to the addition of code coverage tracking (microsoft#187). - Separate the pylint checks (short) from the pytest runs (long, flaky) so that we can have the docker image publish task proceed in nightly runs even if the pytest run was flaky. * Avoid duplicate ctest runs during Github CI runs Fixes microsoft#196 * pass the GITHUB_WORKFLOW env var through the docker process * fixup argument order
Attempting to fix the following error: ``` -> Pinging Codecov https://codecov.io/upload/v4?package=github-action-20201130-cc6d3fe&token=secret&branch=main&commit=27c39e71a07b063cc2c59a3eb72609a722d17510&build=397032896&build_url=http%3A%2F%2Fgithub.com%2Fmicrosoft%2FMLOS%2Factions%2Fruns%2F397032896&name=&tag=&slug=microsoft%2FMLOS&service=github-actions&flags=unittests&pr=&job=&cmd_args=Q,f,n,F,Z HTTP 400 ERROR: Tokenless uploads are only supported for public repositories on GitHub Actions that can be verified through the Actions API. Please use an upload token if your repository is private and specify it via the -t flag. You can find the token for this repository at the url below on codecov.io (login required): Repo token: https://codecov.io/gh/microsoft/MLOS/settings Documentation: https://docs.codecov.io/docs/about-the-codecov-bash-uploader#section-upload-token ``` See Also: https://github.com/microsoft/MLOS/runs/1489105180?check_suite_focus=true
Avoids the following error:
```
Traceback (most recent call last):
File "/opt/hostedtoolcache/Python/3.9.0/x64/bin/licenseheaders", line 8, in <module>
sys.exit(main())
File "/opt/hostedtoolcache/Python/3.9.0/x64/lib/python3.9/site-packages/licenseheaders.py", line 612, in main
arguments = parse_command_line(sys.argv)
File "/opt/hostedtoolcache/Python/3.9.0/x64/lib/python3.9/site-packages/licenseheaders.py", line 304, in parse_command_line
known_extensions = [ftype+":"+",".join(conf["extensions"]) for ftype, conf in typeSettings.items()]
File "/opt/hostedtoolcache/Python/3.9.0/x64/lib/python3.9/site-packages/licenseheaders.py", line 304, in <listcomp>
known_extensions = [ftype+":"+",".join(conf["extensions"]) for ftype, conf in typeSettings.items()]
KeyError: 'extensions'
Error: Process completed with exit code 1.
```
* Make sure cmake generated makefiles target the right build type config when invoked through cake * remove unnecessary dependency on netcore build for cake builds (it's handled internally by cmake as well)
* avoid broken markdown-link-check action version The following upstream change adds quotes around our list of folders to check which results in passing a bad path to find: gaurav-nelson/github-action-markdown-link-check@53c1dda
* Started moving hyperspheres to a separate objective function * Created a Hypersphere ObjectiveFunction and the tests are passing * Linter Co-authored-by: Adam Smiechowski <adsmiech@microsoft.com>
Upstream now has the fix for the issue we were running into: gaurav-nelson/github-action-markdown-link-check#88 https://github.com/gaurav-nelson/github-action-markdown-link-check/releases/tag/1.0.10 https://github.com/gaurav-nelson/github-action-markdown-link-check/releases/tag/v1
1. Invokes pytest with a `-n auto` argument to make multiple tests run in parallel (num parallel jobs = num cores) 2. Installs pytest-xdist in all our pipelines to make the above possible 3. Shortens the duration of TestOptimizerEvaluator 4. Modifies the tests that use gRPC to attempt to start the service on 100 different ports before giving up. This is the easiest way to make sure that all tests requiring gRPC can run in parallel. The alternative would be to have them all talk to a single instance, but it would turn an easy parallelization problem into a hard one as we'd need to manage the lifetime of that single instance. 5. Relaxes the check in TestSmartCacheWithRemoteOptimizerV3.py as the current one was a bit too ambitious and lead to some flakiness. 6. Puts a band-aid on test_optimization_with_context. microsoft#207 hints at a long-term fix. Co-authored-by: Adam Śmiechowski <adsmiech@microsoft.com> Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
* fixup glob for running long haul tests fixes microsoft#191 * need to wrap wildcards in shell
This PR introduces a multi-objective random forest model, which is essentially a collection of single-objective random forests - each predicting a different objective. This model is not yet exposed as part of the optimizer. Co-authored-by: Adam Śmiechowski <adsmiech@microsoft.com>
|
See also #205 |
…ns (microsoft#197) * Comparison of two different ways to summarize decision tree predictions * removed two obsolete helper functions; corrected typo in formula in intro markdown cell * modified what's considered true observation summary stats to reflect duplication of obs across leaves Co-authored-by: Ed Thayer <edthaye@microsoft.com>
…#211) * Workaround an issue introduced by recent versions of sphinx With sphinx 3.4.0-3.4.2 (released in late December 2020), sphinx generates asserts by the Point module due to invalid keys (e.g. __sphinx_mock__) in the Point. Exception occurred: File "/src/MLOS/source/Mlos.Python/mlos/Spaces/Point.py", line 75, in __getitem__ raise KeyError(f"This Point does not have a value along dimension: {dimension_name}") KeyError: 'This Point does not have a value along dimension: __sphinx_mock__' * A better fix to the sphinx hasattr issue * fixup: raise an appropriate error, instead of returning None https://docs.python.org/3.7/reference/datamodel.html?highlight=__getattr__#object.__getattr__
microsoft#212) This is the next batch of code getting us towards multi-objective optimization. PAST: Previously we have been able to register multiple objectives with the optimizer, but the optimizer wouldn't use them at all. PRESENT: With this PR the optimizer will instantiate a regression model for each of the objectives, but it doesn't yet use the predictions for anything. FUTURE: The next steps will be to write a few multi-objective utility functions so that the optimizer can use them to suggest new configurations. The chief goal of this PR is to integrate the MultiObjectiveHomogeneousRandomForest into the framework. Note that there is no change to the Optimizer's interface. The prediction of the first objective is returned, and all the others are thrown out.
* add manual CI run support * disable use of old github docker package registry
This PR enables multi-objective optimization based on multi-objective homogeneous random forest surrogate model and a probability of improvement utility function. The core of this PR is in `MultiObjectiveProbabilityOfImprovementUtilityFunction.py` which uses Monte Carlo simulation to estimate the probability that a given configuration expands the pareto frontier. To support the above functionality, we need a way of determining whether a point is dominated or not. ParetoFrontier.is_dominated(...) does that trick. One way to track optimizer's progress is to track the change in the volume of the pareto frontier. Measuring that volume precisely appears hard, so we settle for a probably approximately correct answer produced by ParetoFrontier.approximate_pareto_volume() for now. All of the other changes revolve around: 1) Integrating the new UtilityFunction into the framework. 2) Persisting the ParetoFrontier within the Optimizer (needed by the utility function). 3) Testing. 4) Small fixes. **Testing** I added new unit tests for new functionality. I also ran multi-objective optimization on 3 different problems over the weekend to compare it to random search (10 repeats for both guided and random search). Firstly, nothing crashed. Secondly, the results are very promising even without any tuning. The graphs below show the comparison between average pareto volume vs. number of iterations for across 10 runs. 2 objectives:   4 objectives:   More work will be required to: 1) Update the OptimizerEvaluator and OptimizerEvaluationReport to capture multi-objective statistics. 2) Update the gRCP interface to support multi-objective predictions 3) Visualizations, more tests and usability improvements. 4) Other utility functions. But practically, this PR enables multi-objective optimization.
…#216) OptimizerEvaluator now tracks pareto over time, pareto volume over time and multi-objective goodness of fit metrics. OptimizerEvaluationReport can now both write itself to disk and read itself back into memory.
Adding EnvelopedWaves and MultiObjectiveEnvelopedWaves as two new synthetic functions with many useful properties. These are sine waves enveloped by another function, either linear, quadratic or another sine wave. An enveloped sine wave produces complexity for the optimizer that allows us evaluate its behavior on non-trivial problems. Simultaneously, sine waves have the following advantages over polynomials: 1. They have well known optima - even when we envelop the function with another sine wave, as long as we keep their frequencies harmonic, we can know exactly where the optimum is. 2. They cannot be well approximated by a polynomial (Taylor expansion is accurate only locally). 3. For multi-objective problems, we can manipulate the phase shift of each objective to control the shape of the pareto frontier. The way multi-objective enveloped waves work is that we pass the same parameters through 1 or more single-objective enveloped waves functions. One useful property is that we not only know where the optima for individual functions are (maxima of sine are easy to find), but we can also know and control the shape of the pareto frontier, by controlling the phase difference between the individual objectives. For example: a phase difference of 0, means that that the objective functions are overlaid on top of each other and their optima are exactly on top of each other, so the pareto frontier is a single, optimal point Alternatively, the phase difference of quarter-period, introduces a trade-off between the objectives where y0 = sin(x) and y1 = sin(x - math.pi / 2) = -cos(x) which yields a pareto frontier in a shape of a quarter-cirle. Yet another option is to use a phase difference of math.pi. This yields a trade-off between the objectives where: y0 = sin(x) and y1 = sin(x - math.pi) = -sin(x) = -y0 which yields a pareto frontier where a gain in one objective results in an equal loss in the other objective, so the shape of that frontier is a diagonal of a square.
This PR makes it possible to create the new objective function via the ObjectiveFunctionFactory. Existing tests cover this addition.
This really simple change adds a start time and an end time to the optimizer evaluation report, thus allowing us to also know how long the optimization process took.
Adding another factory to the project. We already have a BayesianOptimizerFactory, and an ObjectiveFunctionFactory, and with this PR we add a UtiliyFunctionOptimizerFactory. Some subsequent PRs will contain the remaining UtilityFunctionFactory, and RegressionModelFactory. This will help us unify our testing, make sure that all derived classes expose the same interfaces and exhibit sufficiently similar behavior.
I was working on Random Near Incumbent Optimizer and I noticed that GlowWormSwarmOptimizer was doing something really wrong with the context features. I don't really understand how/if it ever worked with context. So this is a quick fix, where we have the worms keep track of the parameter values (not feature values) and only constructing the features dataframe when calling the utility function. This way, the glow worm swarm optimizer can suggest optimal parameters for a given context, instead of suggesting optimal parameters and context. I really don't know how it ever worked. I suspect it was never able to compute the utility value, and always bailed on the first 'if' producing a de-facto random suggestion.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upstream now has the fixes for the issue we were running into:
gaurav-nelson/github-action-markdown-link-check#88
gaurav-nelson/github-action-markdown-link-check#89
https://github.com/gaurav-nelson/github-action-markdown-link-check/releases/tag/1.0.11
https://github.com/gaurav-nelson/github-action-markdown-link-check/releases/tag/v1