search

17zuoye / pyirt

A python library of IRT algorithm
MIT License
99 stars 54 forks source link

enabled to learn c param for 3PL #17

Closed nakamasato closed 3 years ago

nakamasato commented 5 years ago

Hello,

I enabled to learn the pseudo-guessing parameter c. I hope it's acceptable, even though it might be given in most case.

Thank you in advance.

junchenfeng commented 5 years ago

Hi, Naka. Thank you so much for submitting the PR. As it stands, I cannot merge it for two major reasons.

(1) You did not provide additional test cases for your algorithm, which is the prerequisite for any production code.

(2) Please DO NOT change the default interface. I leave out 3 PL learning on purpose because it usually leads to very slow parameter learning and requires the learner data to cover the left tail of the "skill" distribution (aka those who sucks) for parameter convergence. Therefore, 3PL should leave to those who know what they are doing by actively invoking the spec.

That said, I skimmed through the code without finding any obvious problems. Therefore, I think the code is probably right. Please furnish some tests and revert your change to the default interface, I will be happy (and honored) to merge it.

Again, thank you!

nakamasato commented 5 years ago

Hi JunChenFeng,

Thank you for your quick response. I'll study more about it and when I can prove the modification is proper, I'll change back to the current interface, add some tests for it and update the pull request!

Sincerely,