Add conversion methods between OSCAR and HomotopyContinuation as a julia extension#5758
Add conversion methods between OSCAR and HomotopyContinuation as a julia extension#5758antonydellavecchia wants to merge 35 commits into
Conversation
|
This might become a major pain in the future with versions, since HomotopyContinuation indirectly depends on dependencies of Oscar, like FLINT_jll . This could potentially prevent us from updating FLINT_jll in a timely manner, since we have to wait for the Arblib maintainers (as well as the HomotopyContinuation.jl maintainers). I don't know how I feel about this. Happy to hear what others think about this. Testing extensions is tricky. I think @benlorenz a while ago gave me the advice to directly inject the optional dependencies during CI: https://github.com/thofma/Hecke.jl/blob/master/.github/workflows/CILong.yml#L44-L48. In |
|
This dependency could indeed become an issue. Usually I'm faster at updating Arblib.jl than in this case, but at least one or two weeks should definitely be expected in the general case. So there will likely be some time when Arblib.jl and Nemo are out of sync. Due to the tight coupling with Flint we have relatively strict version bounds for FLINT_jll (I think Nemo does the same?). Historically a more permissive version bound would have been fine. The only breaking change for the purposes of Arblib.jl was the change in layout for matrices in Flint 3.2, but even didn't actually give any issues on the Julia side unless you tried to directly use the internal fields. But since it is difficult to know what things might change in the future I would still be hesitant to use a more permissive version. |
I followed your lines regard the injection, but I cant seem to find the other bits of code you are referring to? |
|
Something like this: I have then some code unter |
|
Some comments from triage:
|
|
Latest version 2.17.2 of HomotopyContinuation.jl supports PrettyTables version 3 and restores compat with julia 1.10. |
|
I've been put in as a reviewer for this and I just want to say that the extension code itself looks fine to me but that does not seem to be the contentious part of this PR. I have no actual understanding of the maintenance burden that these incompatible dependency versions might create in the long term. |
lgoettgens
left a comment
There was a problem hiding this comment.
I think we can get rid of most (maybe even all) of the new stub functions by instead of adding methods to the stub functions adding them to the same-named functions of HC instead. This removes some clutter from the Oscar name space (in particular since the names are not really aligned with our naming guidelines), and IMO removes some duplication.
| @@ -0,0 +1,15 @@ | |||
| using HomotopyContinuation | |||
There was a problem hiding this comment.
shouldn't this file be rather experimental/Extensions/test/runtests.jl?
There was a problem hiding this comment.
then it would run as experimental test which we dont want. I've added the test to the experimental folder now. and have a test in runtest that just check that errors are thrown.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5758 +/- ##
==========================================
+ Coverage 83.93% 84.13% +0.19%
==========================================
Files 753 784 +31
Lines 103376 106547 +3171
==========================================
+ Hits 86773 89638 +2865
- Misses 16603 16909 +306
🚀 New features to boost your workflow:
|
| f4ncgb_jll = "0.300.401" | ||
| julia = "1.10" | ||
| lib4ti2_jll = "1.6.10" | ||
| HomotopyContinuation = "2.19" |
There was a problem hiding this comment.
please keep this list sorted alphabetically
This code will be loaded when HomotopyContinuation.jl is a dependency of the active project.
Conversion code more or less just copied from https://github.com/taboege/OscarHomotopyContinuation
This PR will also resolve taboege/OscarHomotopyContinuation#2 (comment)
In terms of what the extension can handle.
Currently we need HomotopyContinuation 2.15.
Adds a new workflow for testing extensions where the test dependencies for the extensions are injected.
(currently just HomotopyContinuation.jl)