Opened 4 years ago

Closed 4 years ago

#19863 closed enhancement (fixed)

Static error rate channel does not always add as many errors as expected

Reported by: dlucas Owned by:
Priority: major Milestone: sage-7.0
Component: coding theory Keywords:
Cc: jsrn Merged in:
Authors: David Lucas Reviewers: Johan Sebastian Rosenkilde Nielsen
Report Upstream: N/A Work issues:
Branch: 1d29dab (Commits) Commit: 1d29dab8edb290f96099b300708e2f166f26005f
Dependencies: Stopgaps:

Description

I noticed the following:

V = VectorSpace(GF(2), 2)
Chan = channels.StaticErrorRateChannel(V, 1)
for i in range(100):
    c = V.random_element()
    r = Chan(c)
    assert c != r

AssertionError

This is due to a line in the transmit_unsafe methods which replaces values of the transmitted vector by random values picked from the base ring of the transmitted vector. It works with high probability for big rings, but obviously not for my example.

In any case, it does not respect the promised behaviour and thus should be fixed.

Change History (11)

comment:1 Changed 4 years ago by dlucas

  • Branch set to u/dlucas/fix_in_static_error_rate_channel

comment:2 Changed 4 years ago by dlucas

  • Authors set to David Lucas
  • Commit set to 634b762415c5f50d9ed490f8d71f399c4693742f
  • Status changed from new to needs_review

I fixed the bug, this is now open for review.


New commits:

634b762Fixed bug in static error rate channel

comment:3 Changed 4 years ago by jsrn

Could you please introduce R = V.base_ring()?

comment:4 Changed 4 years ago by jsrn

Could you add something like your code as a test? E.g. something like

V = VectorSpace(GF(2), 1000)
Chan = channels.StaticErrorRateChannel(V, 367)
assert (c - Chan.transmit(c)).hamming_weight() == 367

Otherwise, the code looks good, builds fine and passes.

comment:5 Changed 4 years ago by git

  • Commit changed from 634b762415c5f50d9ed490f8d71f399c4693742f to 6821e900f630a5d285fec2283a9be064d1379152

Branch pushed to git repo; I updated commit sha1. New commits:

6821e90Integrated reviewer's comments

comment:6 Changed 4 years ago by dlucas

Done and done. Thanks for the help.

David

comment:7 Changed 4 years ago by jsrn

Oh, I guess the doctest is better written with output instead of assert:

   sage: (c - Chan.transmit(c)).hamming_weight()
   367

comment:8 Changed 4 years ago by git

  • Commit changed from 6821e900f630a5d285fec2283a9be064d1379152 to 1d29dab8edb290f96099b300708e2f166f26005f

Branch pushed to git repo; I updated commit sha1. New commits:

1d29dabRewrote the doctest

comment:9 Changed 4 years ago by dlucas

I agree. Done!

comment:10 Changed 4 years ago by jsrn

  • Reviewers set to Johan Sebastian Rosenkilde Nielsen
  • Status changed from needs_review to positive_review

Cool. Green light.

comment:11 Changed 4 years ago by vbraun

  • Branch changed from u/dlucas/fix_in_static_error_rate_channel to 1d29dab8edb290f96099b300708e2f166f26005f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.