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

set compat to julia >= 1.9 #7

Merged
merged 4 commits into from
Apr 18, 2024
Merged

set compat to julia >= 1.9 #7

merged 4 commits into from
Apr 18, 2024

Conversation

jeremiedb
Copy link
Member

No description provided.

@jeremiedb
Copy link
Member Author

@pat-alt Adding tests on Julia <1.9 revealed that there appear to be issues associated with the Optimisers' explicit parametrization approach. Learning seems essentially to fail when running on Julia <v1.9.
I couldn't find back the discussions around it in Flux while making the transition to Optimisers.jl, only this: FluxML/Flux.jl#1986.

As a initial measure, I'd go with the lower bound to Julia v1.9. If there's a clean way to add back compatibility to Julia v1.6 I'd for sure consider it, but hopefully most potential users are operating on Julia 1.9+.

@pat-alt
Copy link
Contributor

pat-alt commented Apr 12, 2024

@jeremiedb thanks for flagging. I guess this may explain why the current dev version of Flux is also bound to 1.9? Not entirely sure though.

I'll add a note about this to the docs for the NeuroTreeExt in CounterfactualExplanations.jl. And thanks for merging the other PR from yesterday.

@pat-alt
Copy link
Contributor

pat-alt commented Apr 12, 2024

Oh I'm now seeing you've changed that compat entry back here, no worries. I'll run tests for the extension on 1.9+ and add a note. Thanks!

Edit: Actually, I still need NeuroTreeModels in the test environment to test the extension on versions 1.9+. I think that for now this means I just won't be able to include tests for it all, unless I remove backward compatibility from CounterfactualExplanations.jl.

@jeremiedb jeremiedb changed the title add julia 1.6 in tests set compat to julia >= 1.9 Apr 12, 2024
@jeremiedb
Copy link
Member Author

Edit: Actually, I still need NeuroTreeModels in the test environment to test the extension on versions 1.9+. I think that for now this means I just won't be able to include tests for it all, unless I remove backward compatibility from CounterfactualExplanations.jl.

For clarification, is the concern that you want to maintain the compatibility of CounterfactualExplanations.jl with Julia 1.6 and not increase it's lower bound to 1.9?

Wold it be possible to have the NeuroTreeModelsExt only available if Julia >= 1.9, for example by not loading the extension if conditional on julia version? Actually, since the extension capability is introduced with Julia 1.9, I think that just commenting out the inclusion of the extension could do it:

if !isdefined(Base, :get_extension)
    # include("../ext/EvoTreesCUDAExt/EvoTreesCUDAExt.jl") # commenting out the inclusion of the extention because of compat with julia 1.9+
end

@pat-alt
Copy link
Contributor

pat-alt commented Apr 15, 2024

For clarification, is the concern that you want to maintain the compatibility of CounterfactualExplanations.jl with Julia 1.6 and not increase it's lower bound to 1.9?

Yes, exactly.

Wold it be possible to have the NeuroTreeModelsExt only available if Julia >= 1.9, for example by not loading the extension if conditional on julia version?

That's basically what I've done in the end. I've just added a separate local environment in the test folder for NeuroTreeModels and activate that only on >= 1.9: https://github.com/JuliaTrustworthyAI/CounterfactualExplanations.jl/tree/main/test/models/neurotree

So ultimately, no issues anymore on my end.

Thanks!

@jeremiedb jeremiedb merged commit dbfee81 into main Apr 18, 2024
4 checks passed
@jeremiedb jeremiedb deleted the dev branch April 20, 2024 21:06
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.

3 participants