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

fix: make :class col more generic to X type #393

Merged
merged 3 commits into from
Dec 26, 2024

Conversation

MilesCranmer
Copy link
Owner

@ablaom this should fix the issue you noticed with rowtable(X) types.

However this will currently result in type instability for NamedTuple input since I have to collect the columns due to this issue: JuliaAI/MLJModelInterface.jl#214. But it does fix the issue for other types.

Copy link
Contributor

github-actions bot commented Dec 23, 2024

Benchmark Results

master 1840f53... master/1840f531077584...
search/multithreading 17.3 ± 1.4 s 17.4 ± 0.66 s 0.996
search/serial 27.7 ± 0.53 s 28.6 ± 0.87 s 0.969
utils/best_of_sample 1.7 ± 0.92 μs 1.72 ± 0.55 μs 0.988
utils/check_constraints_x10 11.2 ± 2.8 μs 11.2 ± 3 μs 0.994
utils/compute_complexity_x10/Float64 2.08 ± 0.11 μs 2.07 ± 0.12 μs 1
utils/compute_complexity_x10/Int64 2.04 ± 0.12 μs 2.01 ± 0.11 μs 1.01
utils/compute_complexity_x10/nothing 1.45 ± 0.13 μs 1.43 ± 0.13 μs 1.01
utils/insert_random_op_x10 5.89 ± 1.9 μs 5.78 ± 1.9 μs 1.02
utils/next_generation_x100 0.421 ± 0.099 ms 0.353 ± 0.098 ms 1.19
utils/optimize_constants_x10 0.0339 ± 0.0087 s 0.0333 ± 0.0075 s 1.02
utils/randomly_rotate_tree_x10 5.2 ± 0.59 μs 5.19 ± 0.58 μs 1
time_to_load 1.84 ± 0.0064 s 1.86 ± 0.004 s 0.993

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12459338137

Details

  • 13 of 13 (100.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 94.786%

Files with Coverage Reduction New Missed Lines %
src/SymbolicRegression.jl 1 98.3%
Totals Coverage Status
Change from base Build 12441469062: 0.03%
Covered Lines: 3145
Relevant Lines: 3318

💛 - Coveralls

@ablaom
Copy link

ablaom commented Dec 26, 2024

I've asked someone to take a look at the MLJModelInterface issue, which doesn't look too difficult to address. I'll leave it up to you whether you proceed with this PR or not. This isn't urgent from my point of view. If you would like me test if this resolves the issue at #392 then I'll need a PR combining both changes please (eg. merge this and rebase #392). Ping me when you 're ready.

@MilesCranmer MilesCranmer changed the base branch from master to narrow-mlj-scitype December 26, 2024 23:09
@MilesCranmer MilesCranmer merged commit c8a43be into narrow-mlj-scitype Dec 26, 2024
17 checks passed
@MilesCranmer MilesCranmer deleted the handle-rowtable branch December 26, 2024 23:09
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