-
Notifications
You must be signed in to change notification settings - Fork 0
add parameter node #39
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
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.
👍 Nothing warranting functional changes. Though some questions about jax
vs numpy
- don't worry about addressing here but if you agree I'll open an issue for us so we don't forget to address the jax.numpy <- numpy
issue later.
if self.value is None: | ||
msg = "Cannot sample an undetermined parameter node." | ||
raise ValueError(msg) | ||
return np.full(samples, self.value) |
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.
Do we want to return a jax.numpy.full
here? I guess right now it's OK as a numpy
return since that's what our other algorithms are returning too, but we should probably move to using jax.numpy
everywhere for future proofing.
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.
Yes, we should open an issue for this I think. Currently, we have numpy arrays as return types which will need updating in a few places
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.
Co-authored-by: Will Graham <[email protected]>
Co-authored-by: Will Graham <[email protected]>
Resolves #36