GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add user-defined types by eriknw · Pull Request #177 · python-graphblas/python-graphblas

Conversation

@eriknw

@eriknw eriknw commented Mar 15, 2022

Copy link
Copy Markdown
Member

UDTs can be any fixed-length numpy type that doesn't have Python objects.
Basically, UDFs that operate on the UDTs need to be able to be compiled with @numba.njit.

Still lots to do, but many things are already working!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
eriknw added 16 commits March 15, 2022 17:37
@eriknw
WIP: begin adding user-defined types
8cfd869
UDTs can be *any* fixed-length numpy type that doesn't have Python objects.
Basically, UDFs that operate on the UDTs need to be able to be compiled with `@numba.njit`.
@eriknw
UDFs on UDTs! Also, get return type of UDFs from numba.
c3268a5
@eriknw
Add recipes for first, any, eq, etc. on UDTs
b03daea
@eriknw
Merge branch 'main' into udts
1956bc8
@eriknw
Merge branch 'main' into udts
0279bfe
@eriknw
Test UDTs
1e3317f
Also:
- make dtypes hashable
- change operator attributes to use dtypes, not dtype names
- update `==` between UDTs (ignore bytes where appropriate)
@eriknw
Allow positional operators to work on UDTs; also, coverage
9e40381
@eriknw
Aggregators on UDT
77433c4
@eriknw
Merge branch 'main' into udts
758473f
@eriknw
Clean up usage of lookup_dtype and unify
0b898ab
Some usage of `lookup_dtype` is not unnecessary now that ops use dtypes instead of dtype names.
Also, using `get_typed_op` is preferred over `unify` where possible.
@eriknw
Merge branch 'main' into udts
6414b69
@eriknw
A little bit of progress
6e92f4f
@eriknw
Fix failing test
37caac4
@eriknw
Merge branch 'main' into udts
12b9daa
@eriknw
Get UDFs to work on array UDTs
6eeabfa
@eriknw
Coverage
c105b92
@eriknw eriknw changed the title WIP: begin adding user-defined types Add user-defined types Apr 8, 2022
eriknw added 3 commits April 8, 2022 22:06
@eriknw
More coverage; better handling of array subdtypes
1ce8421
@eriknw
More coverage. Anything else needed?
c9e8772
@eriknw
A few finishing touches
21f483b
@eriknw

eriknw commented Apr 9, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Whew!

Okay, I think this is pretty much done. Tests and coverage are as solid as I can reasonably make them.
Nevertheless, a PR this large and invasive is likely to have a bug or oversight or three.
I also don't know how it'll impact dask-grblas (CC @ParticularMiner).

Final thoughts and future possibilities:

  • We could allow default operators to work on array dtypes of normal dtypes (such as np.dtype("(3,)uint8"))
    • For example, plus(A | A) could add subarrays together
    • This could also leverage normal NumPy broadcasting rules for the subarrays (i.e., numba's default)
  • (in)equality checking of UDTs could probably be faster
    • array dtypes could probably be specialized
    • should be benchmarked to determine what is best
  • NumPy string dtypes (such as np.dtype("S5")) are not tested (but I played around with them in my preliminary experiments)
  • Should we require dtypes to be registered? Can things "just work" if given a numpy dtype?
    • In this PR, I kept to the standard register_new and register_anonymout pattern we use elsewhere.
  • Ideally, we should document UDTs better in docstrings and online documentation. (We'll get there someday, but I'm wiped!)

This PR opens up a whole universe of possibilities. Record dtypes and array dtypes support different use cases, and both can be super-important.

Anyway, I think this PR should be merged ASAP, but feedback is welcome for any who are courageous enough to review it (CC @jim22k). Although I think things are pretty good, we can always smooth any rough edges we find once we actually implement workloads that needs UDTs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@eriknw
eriknw merged commit 8e9acb9 into python-graphblas:main Apr 10, 2022
@eriknw eriknw mentioned this pull request Apr 10, 2022
ENH: User-defined types #153
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@eriknw
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.