-
-
Notifications
You must be signed in to change notification settings - Fork 52
Add output_size
#413
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
Add output_size
#413
Conversation
|
Sounds reasonable, but not sure what your comment means for this PR. |
Converted to draft until we are sure this is what SparseConnectivityTracer really needs, see adrhill/SparseConnectivityTracer.jl#234 (comment). |
It is clear now that this would be needed to finish adrhill/SparseConnectivityTracer.jl#234 properly. |
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.
Looking good
Something odd happened here: I merged this but it displays as closed 🤔 @visr didn't close the PR. When merging there was an error message saying something like "the server could not be reached". |
😅 Just make a new PR? |
No need, despite the red closed icon, it is on master as intended. |
Add `output_size`
we need a new version for #413
we need a new version for #413
I wanted to update SparseConnectivityTracer to DataInterpolations v8 in adrhill/SparseConnectivityTracer.jl#234 but got a bit confused.
#396 removes the output dimension type parameter and the old
get_output_dim
. Even though it talks about dimensions it was really thesize
of the output.SparseConnectivityTracer was one of the reasons
output_dim
was added to the API, but this actually was thendims
of the output (perhapsoutput_ndims
would be a better name). But that is not what SparseConnectivityTracer needs. Hence I am proposing to addoutput_size
, such that this code doesn't have to live in SparseConnectivityTracer.