Skip to content

Commit

Permalink
Computation between scalars (python-graphblas#358)
Browse files Browse the repository at this point in the history
* Computation between scalars

* Vanilla ewise_union between scalars

* Handle select with scalars for suitesparse-vanilla backend

* Add `ruff` ISC (flake8-implicit-str-concat), T20 (flake8-print), and PIE (flake8-pie)

* Support awkward version 2; also support 11 more pylint checks

* Revert: ignore pylint not-an-iterable

* Move more config to pyproject.toml; fix awkward tests

* CI: break test section into multiple sections for easier review

* Only test one slow task per CI job

* Better; I knew I would like randomization :)

* Update ruff: one cleanup

* Move more config to pyproject.toml (see python-graphblas#356)

* flake8-simplify actually seems nice

* Simplify ScalarMatMulExpr

* Fix and catch using single back-tic instead of two in RST files

* Support accum for scalars

* `__neg__` and `__invert__` return expressions for scalars too

* Add docstrings to new Scalar methods; add pylint task to CI
  • Loading branch information
eriknw authored Jan 6, 2023
1 parent ec2cad6 commit 501fe6f
Show file tree
Hide file tree
Showing 35 changed files with 1,186 additions and 419 deletions.
68 changes: 44 additions & 24 deletions .github/workflows/test_and_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,29 @@ jobs:
channel-priority: strict
activate-environment: graphblas
auto-activate-base: false
# Randomly test one slow task per job (about 5% chance one group isn't run).
#
# TODO: have build matrix be [slowtask]x[os] and randomly choose [pyver] and [sourcetype].
# This should ensure we'll have full code coverage (i.e., no chance of getting unlucky).
# If important in the future, we could also randomize dependency versions.
- name: RNG for tests
uses: ddradar/[email protected]
id: slowtask
with:
contents: |
pytest_normal
pytest_bizarro
pylint
notebooks
weights: |
1
1
1
1
- name: Update env
run: |
mamba install pytest coverage coveralls=3.3.1 pytest-randomly black \
mamba install pytest coverage coveralls=3.3.1 pytest-randomly \
${{ steps.slowtask.outputs.selected == 'pylint' && 'black pylint' || '' }} \
pandas numba scipy networkx cffi donfig pyyaml awkward
- name: Build extension module
run: |
Expand All @@ -78,58 +98,58 @@ jobs:
pip install --no-deps -e .
- name: Unit tests
run: |
coverage run --branch -m pytest ${{ matrix.cfg.testopts }} -v
coverage run -m pytest ${{ matrix.cfg.testopts }} -v \
${{ steps.slowtask.outputs.selected == 'pytest_normal' && '--runslow' || '' }}
- name: Unit tests (bizarro scalars)
run: |
# Run tests again with Scalars being C scalars by default
find graphblas -type f -name "*.py" -print0 | xargs -0 sed -i -s \
-e '/# pragma: is_grbscalar/! s/is_cscalar=False/is_cscalar=True/g' \
-e '/# pragma: is_grbscalar/! s/is_cscalar = False/is_cscalar = True/g' \
-e '/# pragma: to_grb/ s/is_cscalar=True/is_cscalar=False/g' \
-e '/# pragma: to_grb/ s/is_cscalar = True/is_cscalar = False/g'
coverage run -a --branch -m pytest ${{ matrix.cfg.testopts }} -v
coverage run -a -m pytest ${{ matrix.cfg.testopts }} -v \
${{ steps.slowtask.outputs.selected == 'pytest_bizarro' && '--runslow' || ''}}
git checkout . # Undo changes to scalar default
- name: Miscellaneous tests
if: steps.slowtask.outputs.selected == 'pylint'
run: |
# Test (and cover) automatic initialization
coverage run -a --branch graphblas/tests/test_auto_init.py
coverage run -a --branch graphblas/tests/test_external_init.py
coverage run -a graphblas/tests/test_auto_init.py
coverage run -a graphblas/tests/test_external_init.py
# Test (and cover) lazy module loader
echo "from graphblas.agg import count" > script.py
coverage run -a --branch script.py
coverage run -a script.py
echo "from graphblas import agg" > script.py # Does this still cover?
echo "from graphblas.core import agg" >> script.py
coverage run -a --branch script.py
coverage run -a script.py
# Tests lazy loading of lib, ffi, and NULL in gb.core
echo "from graphblas.core import base" > script.py
coverage run -a --branch script.py
coverage run -a script.py
rm script.py
- name: Auto-generated code check
if: steps.slowtask.outputs.selected == 'pylint'
run: |
coverage run -a --branch -m graphblas.core.automethods
coverage run -a --branch -m graphblas.core.infixmethods
# This step uses `black`
coverage run -a -m graphblas.core.automethods
coverage run -a -m graphblas.core.infixmethods
git diff --exit-code
coverage xml
- name: Pylint (informational only; never fails)
if: steps.slowtask.outputs.selected == 'pylint'
run: pylint --exit-zero graphblas/
- name: Coverage
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
COVERALLS_FLAG_NAME: ${{ matrix.cfg.pyver}}/${{ matrix.cfg.testopts }}
COVERALLS_PARALLEL: true
run: |
coverage xml
coverage report --show-missing
coveralls --service=github
- name: codecov
uses: codecov/codecov-action@v3
# Randomly test notebooks 1/3 of the time
- name: random number
uses: ddradar/[email protected]
id: rand
with:
contents: |
true
false
weights: |
1
2
- name: Notebooks Execution check
if: ${{ steps.rand.outputs.selected == 'true' }}
if: steps.slowtask.outputs.selected == 'notebooks'
run: |
mamba install matplotlib nbconvert jupyter 'ipython>=7'
jupyter nbconvert --to notebook --execute notebooks/*ipynb
Expand Down
14 changes: 10 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ repos:
- flake8==6.0.0
- flake8-comprehensions==3.10.1
- flake8-bugbear==22.12.6
- flake8-simplify==0.19.3
- repo: https://github.com/asottile/yesqa
rev: v1.4.0
hooks:
Expand All @@ -65,13 +66,18 @@ repos:
hooks:
- id: codespell
types_or: [python, rst, markdown]
additional_dependencies: [tomli]
files: ^(graphblas|docs)/
args: ["--ignore-words-list=coo,ba"]
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.0.205
rev: v0.0.212
hooks:
- id: ruff
args: ["--force-exclude"]
args: [--force-exclude]
- repo: https://github.com/sphinx-contrib/sphinx-lint
rev: v0.6.7
hooks:
- id: sphinx-lint
args: [--enable, all, "--disable=line-too-long,leaked-markup"]
- repo: local
hooks:
# Add `--hook-stage manual` to pre-commit command to run (very slow)
Expand All @@ -81,7 +87,7 @@ repos:
language: system
types: [python]
stages: [manual]
args: ["graphblas/"]
args: [graphblas/]
pass_filenames: false
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
Expand Down
6 changes: 3 additions & 3 deletions docs/getting_started/faq.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,23 @@ For simple expressions, the two libraries are very similar.
A += 1
For more complex expressions, however, the two libraries diverge significantly.
`pygraphblas` tends towards the `numpy` style of immediate execution and using
``pygraphblas`` tends towards the ``numpy`` style of immediate execution and using
keyword arguments to affect the output.

.. code-block:: python
# pygraphblas
A.mxm(B.transpose(), mask=A, out=C, accum=FP64.PLUS, semiring=FP64.MIN_PLUS)
`python-graphblas` uses delayed expressions and keeps the output-affecting arguments
``python-graphblas`` uses delayed expressions and keeps the output-affecting arguments
together with the output.

.. code-block:: python
# python-graphblas
C(A, accum=binary.plus) << semiring.min_plus(A @ B.T)
`python-graphblas` also contains additional features, such as the `Recorder` and advanced aggregators.
``python-graphblas`` also contains additional features, such as the ``Recorder`` and advanced aggregators.

What is the performance penalty of writing algorithms with python-graphblas vs writing them directly in C?
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Expand Down
2 changes: 1 addition & 1 deletion docs/getting_started/primer.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ as their start and end point.
Common ways to store a graph
----------------------------

The most common way to store a graph is as an edge list, containing the `(start_node, end_node, weight)`.
The most common way to store a graph is as an edge list, containing the ``(start_node, end_node, weight)``.

The following graph and its edge list are shown below.

Expand Down
2 changes: 1 addition & 1 deletion docs/user_guide/init.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ moment.
Default Initialization
----------------------

If submodules are imported before calling `graphblas.init`, a default initialization occurs.
If submodules are imported before calling ``graphblas.init``, a default initialization occurs.
The default backend is ``suitesparse`` and the default mode is ``nonblocking``.

If ``graphblas.init`` is called after the default initialization with different parameters,
Expand Down
2 changes: 1 addition & 1 deletion docs/user_guide/operations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ Element-wise Intersection
Two identically shaped collections can be intersected element-wise. Locations where only one of the
two collections contains a value will be missing in the output.

The GraphBLAS spec calls this operation `eWiseMult` because it has the same behavior as sparse
The GraphBLAS spec calls this operation ``eWiseMult`` because it has the same behavior as sparse
multiplication when missing values are treated as zero. As a result, ``binary.times`` is the
default operator for element-wise intersection.

Expand Down
5 changes: 5 additions & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ dependencies:
# - flake8
# - flake8-bugbear
# - flake8-comprehensions
# - flake8-print
# - flake8-quotes
# - flake8-simplify
# - gcc
# - gh
# - graph-tool
Expand Down Expand Up @@ -89,6 +92,8 @@ dependencies:
# - ruff
# - scalene
# - snakeviz
# - sphinx-lint
# - sympy
# - vim
# - yesqa
# - zarr
5 changes: 1 addition & 4 deletions graphblas/binary/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ def __getattr__(key):
globals()[key] = rv
if key in _delayed_commutes_to:
other_key = _delayed_commutes_to[key]
if other_key in globals():
other = globals()[other_key]
else:
other = other_key
other = globals().get(other_key, other_key)
rv._commutes_to = other
return rv
raise AttributeError(f"module {__name__!r} has no attribute {key!r}")
Expand Down
4 changes: 2 additions & 2 deletions graphblas/binary/numpy.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ def __dir__():

def __getattr__(name):
if name in _delayed:
func, kwargs = _delayed.pop(name)
rv = func(**kwargs)
delayed_func, kwargs = _delayed.pop(name)
rv = delayed_func(**kwargs)
globals()[name] = rv
return rv
if name not in _binary_names:
Expand Down
71 changes: 44 additions & 27 deletions graphblas/core/automethods.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ def __int__(self):
return self._get_value("__int__")


def __invert__(self):
return self._get_value("__invert__")


def __iter__(self):
return self._get_value("__iter__")

Expand All @@ -105,8 +101,8 @@ def __matmul__(self):
return self._get_value("__matmul__")


def __neg__(self):
return self._get_value("__neg__")
def __ne__(self):
return self._get_value("__ne__")


def __or__(self):
Expand Down Expand Up @@ -349,6 +345,17 @@ def _main():
"name",
"nvals",
"wait",
# For infix
"__and__",
"__or__",
"__rand__",
"__ror__",
# Delayed methods
"apply",
"ewise_add",
"ewise_mult",
"ewise_union",
"select",
}
scalar = {
"__array__",
Expand All @@ -358,8 +365,7 @@ def _main():
"__float__",
"__index__",
"__int__",
"__invert__",
"__neg__",
"__ne__",
"_as_matrix",
"_as_vector",
"_is_empty",
Expand All @@ -369,23 +375,14 @@ def _main():
vector_matrix = {
"S",
"V",
"__and__",
"__contains__",
"__getitem__",
"__iter__",
"__matmul__",
"__or__",
"__rand__",
"__rmatmul__",
"__ror__",
"_carg",
"apply",
"diag",
"ewise_add",
"ewise_mult",
"ewise_union",
"reposition",
"select",
"ss",
"to_coo",
"to_values",
Expand Down Expand Up @@ -415,12 +412,8 @@ def _main():
}
common_raises = set()
scalar_raises = {
"__and__",
"__matmul__",
"__or__",
"__rand__",
"__rmatmul__",
"__ror__",
}
vector_matrix_raises = {
"__array__",
Expand Down Expand Up @@ -463,12 +456,20 @@ def _main():
lines = []
lines.append(" _get_value = automethods._get_value")
for name in sorted(common | scalar):
lines.append(f" {name} = wrapdoc(Scalar.{name})(property(automethods.{name}))")
if name == "name":
lines.append(" name = name.setter(automethods._set_name)")
lines.append(
" name = wrapdoc(Scalar.name)(property(automethods.name))"
".setter(automethods._set_name)"
)
else:
lines.append(f" {name} = wrapdoc(Scalar.{name})(property(automethods.{name}))")
lines.append(" # These raise exceptions")
for name in sorted(common_raises | scalar_raises):
lines.append(f" {name} = Scalar.{name}")
for name in sorted(bad_sugar):
if name == "__imatmul__":
continue
lines.append(f" {name} = automethods.{name}")

thisdir = os.path.dirname(__file__)
infix_exclude = {"_get_value"}
Expand All @@ -490,9 +491,17 @@ def get_name(line):
indent = " "
else:
indent = ""
lines.append(f" {indent}{name} = wrapdoc(Vector.{name})(property(automethods.{name}))")
if name == "name":
lines.append(" name = name.setter(automethods._set_name)")
lines.append(
" name = wrapdoc(Vector.name)(property(automethods.name))"
".setter(automethods._set_name)"
)
else:
lines.append(
f" {indent}{name} = wrapdoc(Vector.{name})(property(automethods.{name}))"
)
if name == "ss":
lines.append(' else:\n ss = Vector.__dict__["ss"] # raise if used')
lines.append(" # These raise exceptions")
for name in sorted(common_raises | vector_matrix_raises):
lines.append(f" {name} = Vector.{name}")
Expand All @@ -513,9 +522,17 @@ def get_name(line):
indent = " "
else:
indent = ""
lines.append(f" {indent}{name} = wrapdoc(Matrix.{name})(property(automethods.{name}))")
if name == "name":
lines.append(" name = name.setter(automethods._set_name)")
lines.append(
" name = wrapdoc(Matrix.name)(property(automethods.name))"
".setter(automethods._set_name)"
)
else:
lines.append(
f" {indent}{name} = wrapdoc(Matrix.{name})(property(automethods.{name}))"
)
if name == "ss":
lines.append(' else:\n ss = Matrix.__dict__["ss"] # raise if used')
lines.append(" # These raise exceptions")
for name in sorted(common_raises | vector_matrix_raises):
lines.append(f" {name} = Matrix.{name}")
Expand Down
Loading

0 comments on commit 501fe6f

Please sign in to comment.