Skip to content

Conversation

@liam-o-marsh
Copy link
Contributor

Requirements: #115

@liam-o-marsh liam-o-marsh requested a review from briling November 14, 2025 14:25
@briling
Copy link
Contributor

briling commented Nov 14, 2025

can you please rebase onto new update-docs?

@liam-o-marsh liam-o-marsh force-pushed the speed-laplacian branch 2 times, most recently from 1459865 to 8a35a1c Compare November 14, 2025 19:02
@briling
Copy link
Contributor

briling commented Nov 14, 2025

@liam-o-marsh thanks!

ruff is not supposed to check setup.py I think, at least in the CI workflow (also there are exclusion rules in the config)

@briling briling mentioned this pull request Nov 14, 2025
@briling
Copy link
Contributor

briling commented Nov 14, 2025

should close #120

@briling briling self-requested a review November 14, 2025 23:36
@briling
Copy link
Contributor

briling commented Nov 14, 2025

@liam-o-marsh what do you think of this suggestion 3ba2079? (i can revert)

@briling
Copy link
Contributor

briling commented Nov 14, 2025

btw why is RAM_BATCHING_SIZE so big?

@liam-o-marsh
Copy link
Contributor Author

@liam-o-marsh what do you think of this suggestion 3ba2079? (i can revert)

looks good to me, thanks!

btw why is RAM_BATCHING_SIZE so big?

I assumed this would mostly run on server hardware, but yeah for highly-multithreaded stuff this won't work
but if this is too low, then it's back to the same performance as the old python-for-loop-over-R1 technique, but with the overhead of managing batch sizes of 1
maybe 1GiB would be more reasonable?
how big is the average R2? 1k samples times 10k features? that's already 80Mb for a single row of R1.

@briling
Copy link
Contributor

briling commented Nov 17, 2025

slatm for qm7 is ~11k features, but we can use the C implementation...
i just noticed that any bigger size in the test slows it down

@briling
Copy link
Contributor

briling commented Nov 17, 2025

2b rebased onto new update-docs

@briling
Copy link
Contributor

briling commented Nov 17, 2025

I think it's ready to be merged?

@briling briling marked this pull request as ready for review November 17, 2025 19:33
@briling
Copy link
Contributor

briling commented Nov 17, 2025

maybe rebase on master first though
and shall we squash?

@liam-o-marsh
Copy link
Contributor Author

sounds good to me!

@briling briling merged commit 559f964 into master Nov 18, 2025
8 checks passed
@briling briling deleted the speed-laplacian branch November 18, 2025 11:42
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