Open Bsingstad opened 1 year ago
Hi @Bsingstad , first of all I have to excuse myself for the very (very) late reply on this PR! big sorry for that!
Overall, your PR looks great, however I need to request some changes. In particular:
your_config.py
and your_model.py
) to a new files with better names, i.e. tf_inception_time_config.py
and inception_time.py
code/reproduce_results.py
as is in order to allow others to reproduce our results. Instead we suggest to create a new file called code/reproduce_inception_time_results.py
code/reproduce_results.py
would train all your models for each of the tasks, i.e. a model designed for form
(conf_tf_inception_form
) is also trained on other tasks. Is this intended, or do you prefer specific models for specific tasks? elif modeltype == "YOUR_MODEL_TYPE"
as a template, instead we would suggest to just add an additional case for your models hereother than that, your PR looks great, I'm curious to do pull request once the issues are resolved. What do you think about our suggestions?
Best, Patrick
Hi @helme
Thank you for your suggestions. I will address them as soon as I can.
Best regards Bjørn-Jostein
Hi @helme
Sorry for taking so long time.
I have now updated the pull request according to your requirements:
inception_time.py
and configs tf_inception_time_config.py
. We also added the original template files your_model.py
and your_configs.py
to our PR.code/reproduce_results.py
to code/reproduce_inception_time_results.py
in my PRModel | All | Diagnostic | Subdiagnostic | Superdiagnostic | Form | Rhythm |
---|---|---|---|---|---|---|
Inception Time (all) | 0.926(08) | |||||
Inception Time (diagnostic) | 0.929(09) | |||||
Inception Time (subdiagnostic) | 0.927(08) | |||||
Inception Time (superdiagnostic) | 0.922(06) | |||||
Inception Time (form) | 0.840(11) | |||||
Inception Time (rhythm) | 0.923(32) |
but we didn't find a way to only train the specific models without doing major changes in your base code.
elif modeltype == "YOUR_MODEL_TYPE"
to scp_experiment.py
and added an additional case for my model: elif modeltype == "inception_time_model":
Hi @Bsingstad, I hope my change requests for this pull request are somehow understandable?
I have now committed the changes based on your last review
In this pull request, we propose a new implementation of Inception Time (Tensorflow) which parameters are optimized using grid search. This work will be further explained in a paper published in ReScience C journal : http://rescience.github.io/