Closed ccoffrin closed 2 years ago
Merging #20 (4f5673f) into main (37c5ba8) will decrease coverage by
0.01%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #20 +/- ##
==========================================
- Coverage 94.58% 94.57% -0.02%
==========================================
Files 5 5
Lines 443 442 -1
==========================================
- Hits 419 418 -1
Misses 24 24
Impacted Files | Coverage Δ | |
---|---|---|
src/ising.jl | 100.00% <100.00%> (ø) |
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 37c5ba8...4f5673f. Read the comment docs.
@ccoffrin I just tried it and this seems good. This will likely make the filtering easier in the Analytics package as well. One thing we may want to consider moving forward is what sort of naming conventions we want to apply between variable names representing states in the form of ising spins, binary vectors, and ints. I think that the convention of spin
for ising spins, bin
or binary
for binary vectors, and state
for integer states makes sense. I'll hold off on merging for now to get your take. This naming convention would not require any changes on this branch
Throughout the code now I do belive we are following the convention of spin
(vector of -1/1), binary
(vector of 0/1) and state
(an integer) as you suggest, so I am good with this convention.
So shall we merge this?
@zmorrell after using the
compute_ising_energy_levels
function a bit I realized that most often one probability wants these ordered from lowest to highest energy. So I propose to chance the return value from a dict to a list of tuples as done in this PR. Thoughts?Do like the names of
energy
andstates
in the tuple?