EntilZha commented 3 years ago

Changes in this PR:

Run Black formatter (first commit)
Implement a training harness
Define input data format, both in final parsed form and a json compatible form (so that others can write customized data parsers to one common format)
Implement a CLI for training using the harness + data format
Add unit tests for training using the training harness (each model is now trained for two epochs and tested for exporting to a file).

EntilZha commented 3 years ago

For the merge conflict, the new reqs file should supercede the old one, it might be detecting a merge conflict since this commit is an ancestor of the commit without that file.

codecov-commenter commented 3 years ago

Codecov Report

Merging #6 (77c5264) into master (20a5a76) will increase coverage by 34.10%. The diff coverage is 72.02%.

@@             Coverage Diff             @@
##           master       #6       +/-   ##
===========================================
+ Coverage   42.88%   76.98%   +34.10%     
===========================================
  Files           9       12        +3     
  Lines         485      591      +106     
===========================================
+ Hits          208      455      +247     
+ Misses        277      136      -141

Impacted Files	Coverage Δ
py_irt/cli.py	`0.00% <0.00%> (ø)`
py_irt/scoring.py	`0.00% <0.00%> (ø)`
setup.py	`0.00% <0.00%> (ø)`
py_irt/models/one_param_logistic.py	`68.22% <69.35%> (+42.51%)`	:arrow_up:
py_irt/models/two_param_logistic.py	`68.54% <70.00%> (+46.41%)`	:arrow_up:
py_irt/training.py	`97.40% <97.40%> (ø)`
py_irt/models/four_param_logistic.py	`91.26% <100.00%> (+65.04%)`	:arrow_up:
tests/test_dataset.py	`100.00% <100.00%> (ø)`
tests/test_fourPL.py	`100.00% <100.00%> (ø)`
tests/test_onePL.py	`100.00% <100.00%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 20a5a76...77c5264. Read the comment docs.

EntilZha commented 3 years ago

Looks like all the tests pass except for 3.9. I'm fairly certain is because my pedroai package depends on a simd implementation of json, which there might not be wheels for yet (hence the pip install failure).

I can remove references to that package or leave, just lmk your preference

jplalor commented 3 years ago

Looks like all the tests pass except for 3.9. I'm fairly certain is because my pedroai package depends on a simd implementation of json, which there might not be wheels for yet (hence the pip install failure).

I can remove references to that package or leave, just lmk your preference

Is pedroai just for the json read in? I didn't see it used elsewhere. Also is that the cause of the additional dependencies? If so maybe we refactor the data IO to keep everything fairly lightweight.

EntilZha commented 3 years ago

I vendored the json parsing code into py_irt/io.py, so the issue should be resolved now (and switched from simdjson to regular json; I've tended towards simdjson to speed up gigantic json files, which we don't have here) https://github.com/nd-ball/py-irt/pull/6/commits/8283a7cb11759307db5d1911c5b847d68f757104

nd-ball / py-irt

Add CLI for training and harness for training with dataset format #6

Codecov Report