[FEATURE] Add AKT model - Githubissues

bigdata-ustc / EduKTM

The Model Zoo of Knowledge Tracing Models

Apache License 2.0

188 stars 62 forks source link

[FEATURE] Add AKT model #6

Closed sone47 closed 3 years ago

sone47 commented 3 years ago

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

(Brief description on what this PR is about)

What does this implement/fix? Explain your changes.

add AKT and relevant files

Pull request type

[ ] [DATASET] Add a new dataset
[ ] [BUGFIX] Bugfix
[x] [FEATURE] New feature (non-breaking change which adds functionality)
[ ] [BREAKING] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] [STYLE] Code style update (formatting, renaming)
[ ] [REFACTOR] Refactoring (no functional changes, no api changes)
[ ] [BUILD] Build related changes
[ ] [DOC] Documentation content changes
[ ] [Sync] Synchronization with a repository
[ ] [OTHER] Other (please describe):

Changes

Add AKT model
Add AKT example
Add AKT doc
Add tests for AKT

Does this close any currently open issues?

N/A

Any relevant logs, error output, etc?

N/A

Checklist

Before you submit a pull request, please make sure you have to following:

Essentials

[x] PR's title starts with a category (e.g. [BUGFIX], [FEATURE], [BREAKING], [DOC], etc)
[x] Changes are complete (i.e. I finished coding on this PR)
[x] All changes have test coverage and al tests passing
[x] Code is well-documented (extended the README / documentation, if necessary)
[x] If this PR is your first one, add your name and github account to AUTHORS.md

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

tswsxk commented 3 years ago

Some build changes have been made into main branch, please synchronize with the main branch

tswsxk commented 3 years ago

Test not passed, fix the errors

sone47 commented 3 years ago

Why load_data.py is omitted?

It is copied from popular project AKT which is reviewed by many people I think, and what more important is it only used when convert raw csv data to txt data, so the test only for model can't cover it. But I think it is necessary to put load_data.py in the directory AKT, because it can translate txt data to data form that the AKT model requires. According to the above reasons, I make load_data.py omitted.

tswsxk commented 3 years ago

Make sense, but a good choice is to make a new directory named scripts, and put the load_data.py under a sub-directory named AKT (i.e., scripts/AKT/load_data.py) under it (also, you need to modify pytest.ini, where scripts should be added to norecursedirs), where a README.md (possible some examples) is strongly recommended.

tswsxk commented 3 years ago

Just for sure, the load_data.py is used in nowhere, such as examples?

sone47 commented 3 years ago

Just for sure, the load_data.py is used in nowhere, such as examples?

It is used in examples for converting txt data to model used data.

tswsxk commented 3 years ago

Well, in this case, put load_data.py under examples/AKT