Skip to content

Pre-kompile K definitions and specs for integration tests #2417

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

Merged
merged 31 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0746d93
Towards in-advance kompilation of definitions in tests
geo2a May 7, 2024
0a01455
Allow a permanent prekompiled directory
geo2a May 7, 2024
ee39882
Run less tests, for debugging purposes
geo2a May 7, 2024
79bc8c5
Add test_kompile_targets
geo2a May 7, 2024
1fbf00d
Add file lock
geo2a May 7, 2024
0d01474
De-dupe targets in a smarter way
geo2a May 7, 2024
b91a09b
Don't prekompile definitions for failing tests
geo2a May 7, 2024
9ccd0a8
Add __eq__ override to Target
geo2a May 7, 2024
37b10fd
Use target name as cache key, add cache debug msgs
geo2a May 7, 2024
870f723
Actually read cache from disk
geo2a May 7, 2024
cb0e941
Revert "Run less tests, for debugging purposes"
geo2a May 7, 2024
00bb4b9
Remove debugging messages
geo2a May 7, 2024
0056a78
Correct path stuff
geo2a May 7, 2024
792b7c1
Consolidate conftest.py
geo2a May 7, 2024
c49ab2a
Add comments
geo2a May 7, 2024
9086efd
Add pytest marker for prekompilation
geo2a May 7, 2024
5d8f909
Add more comments
geo2a May 7, 2024
aada391
Skip prekompiling in the same way as proving
geo2a May 7, 2024
4a124e0
Remove Target.__eq__, improve comments
geo2a May 8, 2024
b30fd19
Set Version: 1.0.546
rv-auditor May 8, 2024
3701ddc
Merge branch 'master' into georgy/test-prove-kompile-in-advance
geo2a May 8, 2024
ef47d11
Skip kompilation cache test
geo2a May 8, 2024
5ce032d
Skip prekompilation test if no --kompiled-targets-dir is given
geo2a May 8, 2024
3a9d51f
Code style
geo2a May 8, 2024
0ab98ac
Add smtlib markers to Word operations
geo2a Nov 2, 2023
75852dc
Merge branch 'master' into georgy/test-prove-kompile-in-advance
geo2a May 8, 2024
6981c85
Revert "Add smtlib markers to Word operations"
geo2a May 8, 2024
54c5c4a
Remove redundant parameter
geo2a May 9, 2024
87b7891
Revert "Remove redundant parameter"
geo2a May 9, 2024
ed881e7
Merge remote-tracking branch 'origin/master' into georgy/test-prove-k…
geo2a May 10, 2024
19c4b81
Set Version: 1.0.548
rv-auditor May 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kevm-pyk/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api"

[tool.poetry]
name = "kevm-pyk"
version = "1.0.547"
version = "1.0.548"
description = ""
authors = [
"Runtime Verification, Inc. <[email protected]>",
Expand Down
3 changes: 1 addition & 2 deletions kevm-pyk/src/kevm_pyk/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@
if TYPE_CHECKING:
from typing import Final


VERSION: Final = '1.0.547'
VERSION: Final = '1.0.548'
19 changes: 18 additions & 1 deletion kevm-pyk/src/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
from typing import TYPE_CHECKING

import pytest
from pyk.cli.utils import dir_path

if TYPE_CHECKING:
from pytest import FixtureRequest, Parser
from pathlib import Path

from pytest import FixtureRequest, Parser, TempPathFactory


def pytest_addoption(parser: Parser) -> None:
Expand All @@ -27,6 +30,11 @@ def pytest_addoption(parser: Parser) -> None:
type=str,
help='Run only this specific specification (skip others)',
)
parser.addoption(
'--kompiled-targets-dir',
type=dir_path,
help='Use pre-kompiled definitions for proof tests',
)


@pytest.fixture
Expand All @@ -42,3 +50,12 @@ def use_booster(request: FixtureRequest) -> bool:
@pytest.fixture(scope='session')
def spec_name(request: FixtureRequest) -> str | None:
return request.config.getoption('--spec-name')


@pytest.fixture(scope='session')
def kompiled_targets_dir(request: FixtureRequest, tmp_path_factory: TempPathFactory) -> Path:
dir = request.config.getoption('--kompiled-targets-dir')
if dir:
return dir
else:
return tmp_path_factory.mktemp('prekompiled')
88 changes: 71 additions & 17 deletions kevm-pyk/src/tests/integration/test_prove.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing import TYPE_CHECKING, NamedTuple

import pytest
from filelock import FileLock
from pyk.prelude.ml import is_top
from pyk.proof.reachability import APRProof

Expand All @@ -21,7 +22,7 @@
from typing import Any, Final

from pyk.utils import BugReport
from pytest import LogCaptureFixture, TempPathFactory
from pytest import FixtureRequest, LogCaptureFixture


sys.setrecursionlimit(10**8)
Expand Down Expand Up @@ -118,31 +119,58 @@ class Target(NamedTuple):
main_file: Path
main_module_name: str

@property
def name(self) -> str:
"""
Target's name is the two trailing path segments and the main module name
"""
return f'{self.main_file.parts[-2]}-{self.main_file.stem}-{self.main_module_name}'

def __call__(self, output_dir: Path) -> Path:
return kevm_kompile(
output_dir=output_dir / 'kompiled',
target=KompileTarget.HASKELL,
main_file=self.main_file,
main_module=self.main_module_name,
syntax_module=self.main_module_name,
debug=True,
)
with FileLock(str(output_dir) + '.lock'):
return kevm_kompile(
output_dir=output_dir / 'kompiled',
target=KompileTarget.HASKELL,
main_file=self.main_file,
main_module=self.main_module_name,
syntax_module=self.main_module_name,
debug=True,
)


@pytest.fixture(scope='module')
def kompiled_target_cache(kompiled_targets_dir: Path) -> tuple[Path, dict[str, Path]]:
"""
Populate the cache of kompiled definitions from an existing file system directory. If the cache is hot, the `kompiled_target_for` fixture will not containt a call to `kompile`, saving an expesive call to the K frontend.
"""
cache_dir = kompiled_targets_dir / 'target'
cache: dict[str, Path] = {}
if cache_dir.exists(): # cache dir exists, populate cache
for file in cache_dir.iterdir():
if file.is_dir():
# the cache key is the name of the target, which is the filename by-construction.
cache[file.stem] = file / 'kompiled'
Copy link
Contributor

Choose a reason for hiding this comment

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

Initializing a cache might be unnecessary, you can just do the directory lookup when you request the kompiled directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you mean: could you elaborate?

My goals are:

  • keep using cache in so that the old behavior of the kompiled_target_for fixture is retained,
  • make sure that cache is never actually updated when --kompiled-targets-dir is passed.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I see it, tests basically require an interface that takes a Target and returns a Path to the kompiled target (in the actual tests there might be another indirection from spec files to targets, but I'll not consider this for simplicity).

So you just have to ensure that Target -> Path is reproducible between runs, then accommodating both use cases (prekompiled vs on-the-fly) is straightforward.

  • If the Path does not exist, lock and kompile
  • Otherwise, return the Path.

For prekompiled mode, you pass in a non-empty directory as base. For on-the-file mode, you pass in an empty temporary directory.

cache, etc. can be dropped, the actual cache is the file system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sound good, I understand what you mean. However, I'd rather get my incremental change merged quickly and now spend too much time refactoring the test suite. Would that be OK if I let you refactor it as you see fit?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can live with that, but please open an issue for the refactoring and assign it to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will do!

else:
cache_dir.mkdir(parents=True)
return cache_dir, cache
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider abstracting this in a class.



@pytest.fixture(scope='module')
def kompiled_target_for(tmp_path_factory: TempPathFactory) -> Callable[[Path], Path]:
cache_dir = tmp_path_factory.mktemp('target')
cache: dict[Target, Path] = {}
def kompiled_target_for(kompiled_target_cache: tuple[Path, dict[str, Path]]) -> Callable[[Path], Path]:
"""
Generate a function that returns a path to the kompiled defintion for a given K spec. Invoke `kompile` only if no kompiled directory is cached for the spec.
"""
cache_dir, cache = kompiled_target_cache

def kompile(spec_file: Path) -> Path:
target = _target_for_spec(spec_file)

if target not in cache:
output_dir = cache_dir / f'{target.main_file.stem}-{len(cache)}'
output_dir.mkdir()
cache[target] = target(output_dir)
if target.name not in cache:
Copy link
Contributor

Choose a reason for hiding this comment

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

Target already uniquely identifies a kompilation target, it's probably not necessary to change the key to a str derived from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not possible to construct a Target when initializing the cache from a directory in kompiled_target_cache, as I have nothing to construct the main_file field from.

output_dir = cache_dir / target.name
output_dir.mkdir(exist_ok=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this to cover the case when the user points to an already existing cache to rekompile it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, exist_ok=True is indeed unnecessary as the directory should never exist if the target is not already cached.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, turns out it is actually easier if we keep exist_ok=True, and let the tests wait on the lock to populate/read it, rather then fail to create it if it exists.

cache[target.name] = target(output_dir)

return cache[target]
return cache[target.name]

return kompile

Expand All @@ -156,6 +184,32 @@ def _target_for_spec(spec_file: Path) -> Target:
return Target(main_file, main_module_name)


@pytest.mark.parametrize(
'spec_file',
ALL_TESTS,
ids=[str(spec_file.relative_to(SPEC_DIR)) for spec_file in ALL_TESTS],
)
def test_kompile_targets(spec_file: Path, kompiled_target_for: Callable[[Path], Path], request: FixtureRequest) -> None:
"""
This test function is intended to be used to pre-kompile all definitions,
so that the actual proof tests do not need to do the actual compilation,
which is disturbing performance measurment.

To achieve the desired caching, this test should be run like this:
pytest src/tests/integration/test_prove.py::test_kompile_targets --kompiled-targets-dir ./prekompiled

This test will be skipped if no --kompiled-targets-dir option is given
"""
dir = request.config.getoption('--kompiled-targets-dir')
if not dir:
pytest.skip()

if spec_file in FAILING_BOOSTER_TESTS:
pytest.skip()

kompiled_target_for(spec_file)


# ---------
# Pyk tests
# ---------
Expand Down
2 changes: 1 addition & 1 deletion package/version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.0.547
1.0.548