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

adapt to array changes in julia nightly #732

Merged
merged 2 commits into from
Dec 13, 2023
Merged

adapt to array changes in julia nightly #732

merged 2 commits into from
Dec 13, 2023

Conversation

benlorenz
Copy link
Member

@benlorenz benlorenz commented Nov 21, 2023

This needs JuliaInterop/libcxxwrap-julia#137 which is not released yet.
Also requires some parts of JuliaLang/julia#52248 (depending on how this gets merged the #if can be removed again).

I removed an unused Int32 case from jl_array_to_intvec, this is only used with an explicit Int64 vector anyway.

Locally (on Linux) this works fine with julia 1.6.7, 1.9.4 and nightly with my julia PR and a custom libcxxwrap + cxxwrap.

Note that this branch was based on 0.19 instead of the latest master to allow testing with Oscar. rebased on master.

@fingolfin
Copy link
Member

@benlorenz is this ready now? It would be good to restore compat with Julia nightly (even if we remove parts of this PR again later, once/if Julia merges your PR...)

@benlorenz
Copy link
Member Author

At least some parts of the julia PR are required for this to work. Currently only a function for creating 1-dimensional arrays is exported (jl_alloc_array_1d). The _2d and _3d variants were removed and replaced with _nd, but that is not exported. The PR adds the missing export for _nd and re-adds the _2d and _3d helpers.

When I wrote and tested the code here I just added the _nd export to julia and thus needed the #if block for compatibility. If the 2d and 3d version are merged as well we can remove the #if and revert to the old code.

@benlorenz benlorenz marked this pull request as ready for review December 5, 2023 09:04
@benlorenz
Copy link
Member Author

This does seem to work for nightly now, to build the jll we still need JuliaPackaging/Yggdrasil#7757.

@benlorenz
Copy link
Member Author

I think this can be merged now and then we can create a new libsingular_julia_jll. (libjulia_jll is done)

@benlorenz benlorenz requested a review from fingolfin December 13, 2023 08:20
@fingolfin fingolfin merged commit 2f97ab9 into master Dec 13, 2023
12 of 14 checks passed
@fingolfin fingolfin deleted the bl/julianightly branch December 13, 2023 13:23
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.

2 participants