ilupp
ilupp copied to clipboard
Int64 take2
Here is my attempt at implementing the two-module solution. It is almost entirely transparent to the user and does not require any upcasting. The only difficulty is supporting the iluplusplus_precond_parameter and preprocessing_sequence objects as these are not constructed with a sparse matrix as input and thus have no way of automatically determining the appropriate module to use. My current work-around requires the user to specify the type manually for use with 64bit matrices.
This looks good so far. For the precond_parameter class, most of the Integer
fields there are actually just de-facto enums and could simply be int
. For the remaining ones, it should be fine to use int64 in both instances of the library, then the same class can be used in both versions.
I agree that it would be nice to have only a single version of the parameter class. After re-reading the pybind11 documentation again (this is the relevant page: https://pybind11.readthedocs.io/en/stable/advanced/classes.html) it may actually be quite easy to get this to work. Note that I had to add py::module_local
to all the bindings to be able to compile them into two different modules. While this means we have two different parameter classes on the Python side, both versions are actually accepted by the bindings when passing from Python to C++. Right now, using mismatched parameter classes just results in an error due to the mismatched types. However, if we make sure that the parameter classes are exactly equivalent, whether the library is compiled with 64bit indices or not, this should all just work.
So, changing all the Integer
fields in the parameters to be hard-coded to either int
or int64
and then importing one of the two versions in the python code should make everything work exactly as before and make the switch to 64bit indices completely transparent to the user.
So, changing all the
Integer
fields in the parameters to be hard-coded to eitherint
orint64
and then importing one of the two versions in the python code should make everything work exactly as before and make the switch to 64bit indices completely transparent to the user.
I agree! That's exactly what I had in mind. Is that something you are interested in doing? I find the current state of the PR a bit too messy from the user's perspective, with having to specify the index type manually, but if that could be made transparent again, I would definitely merge it.