byu-dml / d3m-dynamic-neural-architecture

1 stars 1 forks source link

Update Tuning #171

Closed orionw closed 5 years ago

orionw commented 5 years ago
codecov-io commented 5 years ago

Codecov Report

Merging #171 into develop will decrease coverage by 0.15%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #171      +/-   ##
===========================================
- Coverage    56.81%   56.66%   -0.16%     
===========================================
  Files           33       33              
  Lines         2223     2229       +6     
===========================================
  Hits          1263     1263              
- Misses         960      966       +6
Impacted Files Coverage Δ
dna/__main__.py 37.19% <0%> (-0.7%) :arrow_down:

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 3936d8e...9214cae. Read the comment docs.

bjschoenfeld commented 5 years ago

@orionw Are there no changes that need to be made to this package as a result of the update?

orionw commented 5 years ago

@bjschoenfeld No, it should fix the output bug. If you'd like to make a change to the constructor args to add in the results dir, we could do that too now that it's implemented. I wasn't exactly sure what you wanted there.

It should run out of the box as it is now though.

bjschoenfeld commented 5 years ago

@orionw tuning.sh doesn't run as-is. Its a simple fix, but you have to run your code before you commit, let alone make a PR. Having tests for this code would help.

orionw commented 5 years ago

At the time I commented that, it did run out of the box. All I had changed was the requirements file which was a non-breaking change with the tuningdeap repo. I agree that tests would help, but we also have a week til the paper deadline

EDIT: tuningdeap is also non-deterministic so it would be hard to test other than - it runs. Which is a fine enough test but not very specific.