-
Notifications
You must be signed in to change notification settings - Fork 59
[BUGFIX] Limit the range of parameters in IRT and MIRT #33
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
Codecov Report
@@ Coverage Diff @@
## main #33 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 32 32
Lines 1148 1162 +14
=========================================
+ Hits 1148 1162 +14
Continue to review full report at Codecov.
|
|
coverage should be 100% |
EduCDM/IRT/GD/IRT.py
Outdated
| else: | ||
| a = F.softplus(a) | ||
| if torch.max(theta != theta) or torch.max(a != a) or torch.max(b != b): | ||
| raise Exception('Error:theta,a,b may contains nan! The value_range or a_range is too large.') |
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.
Use ValueError
EduCDM/MIRT/MIRT.py
Outdated
| a = F.softplus(a) | ||
| b = torch.squeeze(self.b(item), dim=-1) | ||
| if torch.max(theta != theta) or torch.max(a != a) or torch.max(b != b): | ||
| raise Exception('Error:theta,a,b may contains nan! The a_range is too large.') |
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.
Use ValueError
|
|
||
| def test_train_with_large_range(data, conf, tmp_path): | ||
| user_num, item_num = conf | ||
| try: |
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.
Use pytest.raise to test exception
|
Still, use |
|
The description is not intact @ViviHong200709, you should breifly describe what is changed in |
Thanks for sending a pull request!
Please make sure you click the link above to view the contribution guidelines,
then fill out the blanks below.
Description
Limit the range of parameters in IRT and MIRT
What does this implement/fix? Explain your changes.
...
Pull request type
Changes
EduCDM/EduCDM/IRT/GD/IRT.py: In IRT,Use value_range to limit the range of theta and b.Also use sigmoid(c) make sure that c is limit to [0,1].
EduCDM/EduCDM/MIRT/MIRT.py, EduCDM/EduCDM/MIRT/MIRT.py: In IRT and MIRT,Use a_range to limit the range of a. If a_range is not given ,use softplus(a) to make sure a>0.
Does this close any currently open issues?
#31
Any relevant logs, error output, etc?
no
Checklist
Before you submit a pull request, please make sure you have to following:
Essentials
Comments