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

Add AdjacencyList python wrapper #3509

Merged
merged 12 commits into from
Nov 29, 2024
Merged

Conversation

schnellerhase
Copy link
Contributor

@schnellerhase schnellerhase commented Nov 13, 2024

Introduces a python wrapper for the multi-type exported AdjacencyList.

Addresses (partially) #3483

@schnellerhase schnellerhase changed the title AdjacencList wrapper AdjacencyList wrapper Nov 13, 2024
@schnellerhase schnellerhase changed the title AdjacencyList wrapper Add AdjacencyList python wrapper Nov 14, 2024
@schnellerhase schnellerhase force-pushed the adjlist_wrapper branch 10 times, most recently from bf1b8a1 to 3466313 Compare November 16, 2024 11:44
@schnellerhase schnellerhase marked this pull request as ready for review November 16, 2024 12:32
is_32bit = data.dtype == np.int32
cpp_t = _cpp.graph.AdjacencyList_int32 if is_32bit else _cpp.graph.AdjacencyList_int64
cpp_object = cpp_t(data, offsets) if offsets is not None else cpp_t(data)
return AdjacencyList(cpp_object)
Copy link
Member

Choose a reason for hiding this comment

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

So we need a guard for invalid input style (ie np.floating), or do we think the general nanobind error is sufficient? Usually the nanobind type arrays aren’t very expressive when it comes to dtype.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defending against such calls would require quite a bit of additional logic. It doesn't end there if we did that, then we would need for example also checks of the shapes of the arrays.

Given that it can not cause a false positive, i.e. if we call with a float array it will not result in a working code path that produces bad results, and type hints being present, that should flag this in a modern IDE or with mypy, I think we should stick to the simplicity of not dealing with it.

@jhale jhale added this pull request to the merge queue Nov 29, 2024
Merged via the queue into FEniCS:main with commit 11f485e Nov 29, 2024
20 checks passed
@schnellerhase schnellerhase deleted the adjlist_wrapper branch December 4, 2024 16:44
schnellerhase added a commit to schnellerhase/fenics-dolfinx that referenced this pull request Dec 28, 2024
* Add adjacency list python wrapper

* Fix type hint

* Add docstrings

* Apply suggestions from code review

Co-authored-by: Jørgen Schartum Dokken <[email protected]>

* Ruff

* Add TODO:

* Fix default is int64 behavior

* Apply suggested docstring

---------

Co-authored-by: Jørgen Schartum Dokken <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants