Closed jschueller closed 1 year ago
The first and foremost goal of the Python binding should be https://github.com/scipy/scipy/issues/18118. So we have to respect the coding style and rules of the SciPy community, and keep communicating with them. I will invite them to this conversation.
- agreed, it is only there waiting for a validated pure Python implementation
Sorry, I do not think we are talking about the same thing --- we are talking about the opposite.
The first and foremost goal of the Python binding should be integrating the Fortran version of PRIMA into SciPy.
Yes, there will be eventually a Python implementation, but I do foresee a mature Python implementation will be possible in the near future (e.g., within 2 years, if not longer) given the high complexity of these algorithms. However, the community should not continue to use the buggy F77 version for such a long time.
I have worked on these solvers in all the details, so I know how hard they are. As a reference, Tom has been developing a solver of the same kind named COBYQA for exactly two years (the first commit was on 2 Sept 2021), full time. Tom is quite capable and diligent, but I do not think COBYQA is mature yet (sorry Tom, it is close).
In addition, a new implementation should go through extensive verification and tests before it is safe to use in production (recall how many users SciPy has), which will take time.
That said, the Fortran-Python binding is quite important. It is not something temporary. For the interest of the SciPy community, the pure Python implementation will have to achieve the same performance and robustness as the modern Fortran version before it takes the place of the latter.
Thanks.
the python binding is not compiled, cmake is just used to copy prima.py and run the examples
Probably we are not talking about the same thing again. What I mean is the building system for compiling the Fortran code and making it callable in Python. The SciPy community has announced that they will move to Meson (see also comments from the Fortran community). We have to use Meson as the building system for the Python binding if we want it to be accepted by SciPy.
Thanks.
we dont have to move to meson, integrating into scipy will consist in copying the sources there into their own build system, and even if we moved to meson I doubt much would be reusable anyways.
to use the fortran libs directly the iso_c_binding part should be moved inside the fortran lib, but I dont think it would affect the performance nor the maintenance since the c binding would probably have to keep in sync with the fortran side anyways, as for performance its only for IO, so I dont think it matters
Sorry, I believe this matters a lot, especially to the SciPy maintainers. A mixture of Fortran and Python is already considered unpleasant. Getting another language involved will make things much more complicated --- maybe not to ourselves, but remember that other people may have to maintain our code in the (not very far) future.
Above all, it is not necessary to traverse the C interface if Python wants to communicate with iso_c_binding Fortran subroutines. So I do not see why the C interface should be invited into the play.
to use the fortran libs directly the iso_c_binding part should be moved inside the fortran lib
Is it the compiled version or the source code that has to be moved?
Thanks.
we dont have to move to meson, integrating into scipy will consist in copying the sources there into their own build system, and even if we moved to meson I doubt much would be reusable anyways.
Sorry, this is beyond my knowledge again. So we do not need to provide or contribute to the building facility that "compiles the Fortran code and makes it callable in Python", in order to get Python binding integrated into SciPy?
I thought preparing such a building system and ensuring it works on all platforms was the most challenging part.
I guess all the c/bobyqa_c.f90, c/cintrf.f90, ... source files should be moved to /fortran, else the wont be compiled into libprimaf.so and in turn we wont be able to use it in the Python layer instead of libprimac.so like it is done now
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
ctypeslib fcon fobj maxfunt PYTHONPATH
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
fcon fobj maxfunt
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
fcon
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
fcon
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
fcon
Comments by @andyfaff, copied from https://github.com/scipy/scipy/issues/18118
I quickly glanced over the Python binding code, there's pretty much no comments on it, so it'll harder to go through.
- New solvers being added to scipy should ideally use the
optimize.NonlinearConstraint
/optimize.LinearConstraint
syntax for specifying constraints, and useoptimize.Bounds
for lower/upper bounds.- the build system for scipy uses meson, not cmake.
- if possible the minimiser should provide a
callback
ability to update the user on the progress of the minimisation.- the minimiser should have the ability to add extra user defined args,
f(x, *args)
.- bindings should follow existing approaches, i.e. f2py/cython/..., no SWIG or handwritten C extension modules.
- all fortran code should be re-entrant.
temporarily delete other tests
Please remember to get them back before we merge the PR :) Thank you.
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
julia
Hi @jschueller Julien,
This is for your reference: https://github.com/orgs/libprima/discussions/79 .
I suppose we are currently working on the basic interface defined there, which will be sufficient for the inclusion of PRIMA in SciPy.
Thanks.
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
hanles
yes, lets keep things things basic here I think I might be done here, what do you think ?
Thank you @jschueller Julien for the huge efforts! This is very nice.
Things I have noted
Aineq
should be 2D arrays (what are the most common representations of matrices in Python?). New solvers being added to scipy should ideally use the optimize.NonlinearConstraint/optimize.LinearConstraint syntax for specifying constraints, and use optimize.Bounds for lower/upper bounds.
I am ignorant about Python, so @ragonneau Tom please review.
See the :open_file_folder: files view, the :scroll:action log or :angel: SARIF report for details.
uppper
I prefer to not use scipy object forw now and keep the interface simple (basic interface)
So this interface is not for the integration with SciPy?
its a basic layer, including in scipy will require more functions on top of that
its a basic layer, including in scipy will require more functions on top of that
Will this binding include such functions as well?
no, just the api will change
no, just the api will change
Sorry, I did not get it. Could you elaborate a bit more? How will it change, and how will that facilitate the integration with SciPy? Thank you.
Hi @jschueller ,
Thank you for your immense efforts again!
I am totally ignorant about Python. So I could only give very rough comments, which may very likely be wrong. I am open to discussions.
The first and foremost goal of the Python binding should be the integration of PRIMA solvers to SciPy. So we have to respect the coding style and rules of the SciPy community, and keep communicating with them. I will invite them to this conversation.
For the moment, the first thing on my mind is the building system. According to what I read and my limited understanding, SciPy is adopting Meson (see also comments from the Fortran community). Let us follow.
Avoid the three-language issue. Even though the C interface is extremely nice, we should not use it as a bridge between Fortran and Python (or any other languages). The binding should be directly between Fortran and Python (possibly based on the
iso_c_binding
subroutines under https://github.com/libprima/prima/tree/main/c). There are many reasons, the following being some examples.Many thanks for everything!
Also thank Tom @ragonneau for looking into this PR. Our opinions are similar in most cases thanks to our long-term collaboration.
Best regards, Zaikun