17zuoye / pyirt

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

item_param_dict and last_item_param_dict #14

Closed marcelo-trier closed 5 years ago

marcelo-trier commented 6 years ago

Hi Junchen Feng,

I'm not sure if my comment is a real problem. I just want to contribute your code.

In the file: solver/model.py, in the lines: 256 and 269, you 'change' the values of the variables 'last_item_param_dict' and 'item_param_dict'. https://github.com/17zuoye/pyirt/blob/c8f8c8e0c722b223e5900d932ac7fdba5934b5d8/pyirt/solver/model.py#L256 https://github.com/17zuoye/pyirt/blob/c8f8c8e0c722b223e5900d932ac7fdba5934b5d8/pyirt/solver/model.py#L269

However, the way the code is written, python will keep the same memory region being accessed by these two variables.

If you run id(last_item_param_dict) == id(item_param_dict) ..the answer will be: True.

I am in doubt whether this is a bug, or if you would like to keep these two variables pointing to the same structure in memory?

I did some testing and found that if I make an explicit copy of the dictionaries the result will be (a little) different.

The lines below show my test: line 256: self.item_param_dict = self.last_item_param_dict.copy() line 269: self.last_item_param_dict = self.item_param_dict.copy()

Thank you.

junchenfeng commented 5 years ago

I believe you are right! A deep copy is called for here. As it stands, last_item_param_dict is useless.