-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fill diagonal elements of error_matrix from gradient estimators #85
Fill diagonal elements of error_matrix from gradient estimators #85
Conversation
74237b9
to
2b1f55f
Compare
var = ( | ||
(estimates[2 * i].error ** 2) + (estimates[2 * i + 1].error ** 2) | ||
) / delta | ||
err_diag.append(np.sqrt(var)) |
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 seems to be
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.
Thank you for pointing this out!
I misremembered the propagation rule of error.
The latter one is correct.
@@ -62,7 +64,7 @@ def numerical_gradient_estimates( | |||
Returns: | |||
The estimated values (can be accessed with :attr:`.values`) with errors | |||
of estimation (can be accessed with :attr:`.error_matrix`). Currently, | |||
:attr:`.error_matrix` returns `None`. | |||
non-diagonal elements of :attr:`.error_matrix` are filled with zeros. |
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.
Saying "are filled with zeros" sounds to me like "we don't know the exact values but we put zeros at the moment", but I guess the covariances are exactly zero in this case since each element is estimated indepedently.
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.
I was a bit confused but, as you guessed, the covariance are exactly zero.
So I've removed the sentence.
for p, c in params_and_coefs: | ||
g += estimates_dict[p].value * c | ||
var += (estimates_dict[p].error ** 2) * abs(c) |
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.
I guess
var += (estimates_dict[p].error ** 2) * abs(c) | |
var += (estimates_dict[p].error ** 2) * abs(c) ** 2 |
2b1f55f
to
e4d3868
Compare
@kwkbtr |
In the current implementation, gradient estimators do not return error_matrix.
This PR contains changes that make estimators return a matrix where the diagonal elements are filled with its error.
Note that non-diagonal elements are filled with zeros without calculation.