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

Merge develop into smartsim-refactor #668

Merged
merged 5 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions .github/workflows/run_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ jobs:
- name: Install SmartSim (with ML backends)
run: |
python -m pip install git+https://github.com/CrayLabs/SmartRedis.git@develop#egg=smartredis
python -m pip install .[dev,ml]
python -m pip install .[dev,mypy,ml]

- name: Install ML Runtimes with Smart (with pt, tf, and onnx support)
if: contains( matrix.os, 'ubuntu' ) || contains( matrix.os, 'macos-12')
Expand All @@ -129,7 +129,6 @@ jobs:

- name: Run mypy
run: |
python -m pip install .[mypy]
make check-mypy

# TODO: Re-enable static analysis once API is firmed up
Expand Down Expand Up @@ -174,7 +173,7 @@ jobs:
retention-days: 5

- name: Upload Pytest coverage to Codecov
uses: codecov/codecov-action@v3.1.4
uses: codecov/codecov-action@v4.5.0
with:
fail_ci_if_error: false
files: ./coverage.xml
16 changes: 16 additions & 0 deletions doc/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ To be released at some future point in time

Description

- Update codecov to 4.5.0
- Remove build of Redis from setup.py
- Mitigate dependency installation issues
- Fix internal host name representation for Dragon backend
- Make dependencies more discoverable in setup.py
- Add hardware pinning capability when using dragon
Expand All @@ -27,6 +30,19 @@ Description

Detailed Notes

- Update codecov to 4.5.0 to mitigate GitHub action failure
([SmartSim-PR657](https://github.com/CrayLabs/SmartSim/pull/657))
- The builder module was included in setup.py to allow us to ship the
main Redis binaries (not RedisAI) with installs from PyPI. To
allow easier maintenance of this file and enable future complexity
this has been removed. The Redis binaries will thus be built
by users during the `smart build` step
- Installation of mypy or dragon in separate build actions caused
some dependencies (typing_extensions, numpy) to be upgraded and
caused runtime failures. The build actions were tweaked to include
all optional dependencies to be considered by pip during resolution.
Additionally, the numpy version was capped on dragon installations.
([SmartSim-PR653](https://github.com/CrayLabs/SmartSim/pull/653))
- setup.py used to define dependencies in a way that was not amenable
to code scanning tools. Direct dependencies now appear directly
in the setup call and the definition of the SmartRedis version
Expand Down
52 changes: 2 additions & 50 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@
from pathlib import Path

from setuptools import setup
from setuptools.command.build_py import build_py
from setuptools.command.install import install
from setuptools.dist import Distribution

# Some necessary evils we have to do to be able to use
# the _install tools in smartsim/smartsim/_core/_install
Expand All @@ -95,12 +92,6 @@
buildenv = importlib.util.module_from_spec(buildenv_spec)
buildenv_spec.loader.exec_module(buildenv)

# import builder module
builder_path = _install_dir.joinpath("builder.py")
builder_spec = importlib.util.spec_from_file_location("builder", str(builder_path))
builder = importlib.util.module_from_spec(builder_spec)
builder_spec.loader.exec_module(builder)

# helper classes for building dependencies that are
# also utilized by the Smart CLI
build_env = buildenv.BuildEnv(checks=False)
Expand Down Expand Up @@ -128,41 +119,7 @@
class BuildError(Exception):
pass


# Hacky workaround for solving CI build "purelib" issue
# see https://github.com/google/or-tools/issues/616
class InstallPlatlib(install):
def finalize_options(self):
super().finalize_options()
if self.distribution.has_ext_modules():
self.install_lib = self.install_platlib


class SmartSimBuild(build_py):
def run(self):
feature_store_builder = builder.FeatureStoreBuilder(
build_env(), build_env.MALLOC, build_env.JOBS
)
if not feature_store_builder.is_built:
feature_store_builder.build_from_git(versions.REDIS_URL, versions.REDIS)

feature_store_builder.cleanup()

# run original build_py command
super().run()


# Tested with wheel v0.29.0
class BinaryDistribution(Distribution):
"""Distribution which always forces a binary package with platform name

We use this because we want to pre-package Redis for certain
platforms to use.
"""

def has_ext_modules(_placeholder):
return True

# Define needed dependencies for the installation

extras_require = {
"dev": [
Expand All @@ -182,6 +139,7 @@ def has_ext_modules(_placeholder):
"types-tqdm",
"types-tensorflow==2.12.0.9",
"types-setuptools",
"typing_extensions>=4.1.0",
],
"docs": [
"Sphinx==6.2.1",
Expand Down Expand Up @@ -225,15 +183,9 @@ def has_ext_modules(_placeholder):
"pygithub>=2.3.0",
"numpy<2",
"smartredis>=0.5,<0.6",
"typing_extensions>=4.1.0,<4.6",
],
cmdclass={
"build_py": SmartSimBuild,
"install": InstallPlatlib,
},
zip_safe=False,
extras_require=extras_require,
distclass=BinaryDistribution,
entry_points={
"console_scripts": [
"smart = smartsim._core._cli.__main__:main",
Expand Down
2 changes: 1 addition & 1 deletion smartsim/_core/_cli/scripts/dragon_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def install_package(asset_dir: pathlib.Path) -> int:
logger.info(f"Installing package: {wheel_path.absolute()}")

try:
pip("install", "--force-reinstall", str(wheel_path))
pip("install", "--force-reinstall", str(wheel_path), "numpy<2")
wheel_path = next(wheels, None)
except Exception:
logger.error(f"Unable to install from {asset_dir}")
Expand Down
Loading