Skip to content

Conversation

@londumas
Copy link

Fix issue #70.
The CAZAC sequence was improperly coded, leading to non-zero cross-correlation terms when seq_length was an even number. This is fixed using the formula from https://en.wikipedia.org/wiki/Zadoff%E2%80%93Chu_sequence, thanks to the cf term.
Furthermore, additional checks are done to see if inputs are valid.

@londumas
Copy link
Author

Tests failed, but I bet it is because of runtime, on my computer there were no issues:

<me>/CommPy/commpy/tests$ python test_sequences.py 
.
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK

@BastienTr
Copy link
Collaborator

It make sense when looking at Travis logs. I pushed an empty commit on the master branch to confirm this assumption.

@coveralls
Copy link

coveralls commented Nov 10, 2020

Coverage Status

Coverage increased (+0.4%) to 82.799% when pulling ac51ec6 on londumas:fix-cazac-sequence into bd47de1 on veeresht:master.

@londumas
Copy link
Author

@BastienTr, the travis checks are ok, but now it says coverage is not ok. Do you want me to do anything?

@BastienTr
Copy link
Collaborator

Coveralls is not happy with the coverage decrease. It would be good to add a test in tests/test_sequences.py that checks the value from a known sequence and check that the assertions are properly raised.

Besides, I recommend switching from assert to error raising. As a reference you can have a look to the modulation code, doc and test.

Anyway, thanks for the contribution 👍

@londumas
Copy link
Author

@BastienTr This commit should fix these different issues and recommendations.
I also corrected "test_pnsequence" where two asserts where in a with loop, removing the test of the second test.

@BastienTr BastienTr merged commit 1d2fe18 into veeresht:master Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants