-
Notifications
You must be signed in to change notification settings - Fork 45
Limit size of partials by chunk size #35
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
Conversation
@@ -44,20 +44,34 @@ end | |||
generate_chunked_partials(x,color,N::Integer) = generate_chunked_partials(x,color,Val(N)) | |||
function generate_chunked_partials(x,color,::Val{N}) where N | |||
|
|||
# TODO: should only go up to the chunksize each time, and should | |||
# generate p[i] different parts, each with less than the chunksize | |||
chunksize = getsize(default_chunk_size(maximum(color))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we still aren't making use of the actual chunksize if passed by the user, so we should fix that up in a follow up.
col_index = (i-1)*chunksize + j | ||
J[:, col_index] .= partials(fx, color_i) | ||
color_i += 1 | ||
if color_i == maximum(color) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will only happen at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see what this is. The name is confusing because it's not longer necessarily the color. Name it like cur_partial
. And shouldn't it reset after it hits the chunksize? Otherwise partials(fx, color_i)
can have color_i > chunksize
and it will index a partial that doesn't exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you're right. But I think we can just remove the color_i
variable entirely and simply copy the columns from the partials to the Jacobian using
for j in 1:chunksize
col_index = (i-1)*chunksize + j
J[:, col_index] .= partials.(fx, j)
end
We use col_index
variable to map which column in the original matrix would the the jth
column in the ith
pass of the chunked partials would correspond to, using the equation supplied.
Co-Authored-By: Christopher Rackauckas <[email protected]>
Looks good! Still a few things I would change around, so I'll just merge and then send a PR when my flight is done. |
Allows user to control the number of partials to be computed simultaneously by the program by passing the limit as chunk size.
Reference: #23