Adding Hyvarinen Score Matching Score for the normal distribution #396
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements the Hyvarinen Score Matching Score for the normal distribution in NGBoost, based on the referenced JMLR paper. The implementation provides an alternative scoring method that reportedly achieves similar performance to the existing log score.
Key changes:
- Added a generic
ScoreMatchingScorebase class in the scores module - Implemented
NormalScoreMatchingScorewith score computation, derivatives, and Fisher information matrix - Integrated the new scoring method into the Normal distribution's available scores
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ngboost/scores.py | Adds generic ScoreMatchingScore base class with reference documentation |
| ngboost/distns/normal.py | Implements NormalScoreMatchingScore and adds it to Normal distribution's score options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def score(self, Y): | ||
| loc = self.loc | ||
| var = self.var | ||
| var = var |
There was a problem hiding this comment.
This line is redundant - var is assigned to itself without any transformation. This should be removed.
| var = var |
| n = len(Y) | ||
| D = np.zeros((len(Y), 2)) | ||
| D[:, 0] = (self.loc -Y)/(self.var **2) | ||
| D[:, 1] = 2/self.var - (2*(Y - self.loc)**2)/(self.var**2) |
There was a problem hiding this comment.
[nitpick] The expression (2*(Y - self.loc)**2)/(self.var**2) can be simplified to 2*(Y - self.loc)**2/self.var**2 for better readability.
| D[:, 1] = 2/self.var - (2*(Y - self.loc)**2)/(self.var**2) | |
| D[:, 1] = 2/self.var - 2*(Y - self.loc)**2/self.var**2 |
| loc = self.loc | ||
| var = self.var | ||
| var = var | ||
| return ((Y - loc)**2 / (2* var**2)) - 1/var |
There was a problem hiding this comment.
[nitpick] Inconsistent spacing around the multiplication operator. Should be 2 * var**2 instead of 2* var**2.
| return ((Y - loc)**2 / (2* var**2)) - 1/var | |
| return ((Y - loc)**2 / (2 * var**2)) - 1/var |
| def d_score(self, Y): | ||
| n = len(Y) | ||
| D = np.zeros((len(Y), 2)) | ||
| D[:, 0] = (self.loc -Y)/(self.var **2) |
There was a problem hiding this comment.
[nitpick] Inconsistent spacing: extra space after = and missing space before -Y. Should be D[:, 0] = (self.loc - Y)/(self.var**2).
| D[:, 0] = (self.loc -Y)/(self.var **2) | |
| D[:, 0] = (self.loc - Y) / (self.var**2) |
| return ((Y - loc)**2 / (2* var**2)) - 1/var | ||
|
|
||
| def d_score(self, Y): | ||
| n = len(Y) |
There was a problem hiding this comment.
The variable n is defined but never used in the function. This line should be removed.
| n = len(Y) |
|
Please run make lint and make test in your environment to fix the linting issues causing the build to fail |
2 participants
In this pull request, I added the Hyvarinen Score Matching Score based on the paper http://jmlr.org/papers/v6/hyvarinen05a.html. The performance in terms of point estimation and negative log likelihood is very close to the logScore presented in the paper.