inducer / pymetis

A Python wrapper around Metis, a graph partitioning package
http://mathema.tician.de/software/pymetis
Other
156 stars 32 forks source link

New options handling #28

Closed inducer closed 2 years ago

inducer commented 3 years ago

NB: This currently fails to import with

/usr/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
test_metis.py:28: in <module>
    import pymetis
../pymetis/__init__.py:28: in <module>
    from pymetis._internal import Options
E   ImportError: AttributeError: NUMBERING

when exactly the same method seems to work OK in PyOpenCL:

https://github.com/inducer/pyopencl/blob/0195ba6ab8d5fefddf8907820120c9db2c312527/src/wrap_constants.cpp#L156-L179

nishraptor commented 3 years ago

What can I do to help with this? I was planning on just adding a seed option like the other options, but this would be a good solution to add any options. I've pulled the branch but not sure how I can help

inducer commented 3 years ago

I think the task now would be to actually get it to work: expose the option constants and fix the options passing of the functions to use this new mechanism, update the docs, and add some tests. Do this, or some part of it, stick it in a PR, and I'd call that progress.

nhartland commented 3 years ago

I had a shot at understanding the error here but didn't make any progress. Is it even clear where exactly that AttributeError is thrown?

inducer commented 3 years ago

Thanks for giving this a look. I believe the error is being thrown from this line, specifically the .attr in this macro.

nhartland commented 3 years ago

That part seems to compile fine though. I tried rewriting it a few different ways (with/without macro) to no effect. Indeed removing it stops the error on import though.

I was thinking that the error is thrown somewhere in the python side, but couldn't track down exactly where the raise AttributeError is.

inducer commented 3 years ago

I think I made some progress. af76cad gets option setting/getting to work AFAICT.

nhartland commented 3 years ago

Seems to work! Thanks! I can have a shot at wiring up the options and testing them.

What would be the mechanism for passing the pymetis.Options python object back to C++? Something like adding a const py::object &options_py to the signature of wrap_part_graph and then casting back to a metis_options class with py::cast or is there a simpler way?

inducer commented 3 years ago

See 406d0f4.

inducer commented 2 years ago

Superseded by #31, I think.