codecov-commenter
commented
2 years ago Codecov Report
Merging #35 (62733f4) into main (e22dc35) will decrease coverage by
5.67%. The diff coverage is82.22%.
@@ Coverage Diff @@
## main #35 +/- ##
===========================================
- Coverage 100.00% 94.32% -5.68%
===========================================
Files 9 11 +2
Lines 147 141 -6
===========================================
- Hits 147 133 -14
- Misses 0 8 +8 | Impacted Files | Coverage Δ | |
|---|---|---|
| src/types/empirical_potential.jl | 100.00% <ø> (ø) |
|
| src/types/non_trainable_potential.jl | 0.00% <0.00%> (ø) |
|
| src/empirical/bm.jl | 100.00% <100.00%> (ø) |
|
| src/empirical/coulomb.jl | 100.00% <100.00%> (ø) |
|
| src/empirical/lj.jl | 100.00% <100.00%> (ø) |
|
| src/empirical/morse.jl | 100.00% <100.00%> (ø) |
|
| src/empirical/zbl.jl | 100.00% <100.00%> (ø) |
|
| src/mixed/linear_combination_potential.jl | 100.00% <100.00%> (ø) |
|
| src/types/abstract_potential.jl | 100.00% <100.00%> (ø) |
|
| src/types/trainable_potential.jl | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update e22dc35...62733f4. Read the comment docs.
dallasfoster
This PR is a follow-up to #32. My proposal is to update the hierarchy by branching at the root (
AbstractPotential, renamed fromArbitraryPotentialinto two types:TrainablePotentialandNonTrainablePotential. TheTrainablePotentialautomatically handles all of the parameter manipulation and default serialization logic.NonTrainablePotentialprovides default implementations for potentials such as theDFTKPotentialthat don't want to use our interface. Having these defaults allows us to do clever things to concatenate parameter sets for mixed potentials (not implemented yet). Notice how for each empirical potential all we have to do is specify the names of the parameters and hyperparameters. All functionality is equivalent to how it was before this PR.