Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an LFSR module #5

Merged
merged 3 commits into from
Apr 15, 2024
Merged

Add an LFSR module #5

merged 3 commits into from
Apr 15, 2024

Conversation

AngelEzquerra
Copy link
Contributor

This module, which is heavily based on Nikesh Bajaj's pylsfr (https://pylfsr.github.io), implements a Linear Feedback Shift Register that can be used to generate pseudo-random boolean sequences. It supports both Fibonacci and Galois LFSRs.

LFSRs are used in many digital communication systems (including, for example LTE and 5GNR). For more information see https://simple.wikipedia.org/wiki/Linear-feedback_shift_register

@AngelEzquerra AngelEzquerra force-pushed the lfsr branch 3 times, most recently from 819fe40 to edf1ddf Compare March 25, 2024 14:38
This module, which is heavily based on Nikesh Bajaj's pylsfr (https://pylfsr.github.io), implements a Linear Feedback Shift Register that can be used to generate pseudo-random boolean sequences. It supports both Fibonacci and Galois LFSRs.

LFSRs are used in many digital communication systems (including, for example LTE and 5GNR). For more information see https://simple.wikipedia.org/wiki/Linear-feedback_shift_register
impulse/lfsr.nim Outdated

# Remove the last value from taps if it is a zero
let taps = if taps[taps.size - 1] == 0: taps[_..^2] else: taps
if taps.size > 1 and taps[0] < taps[taps.size - 1]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this mean for multiple polynomials an ordering like [7, 3, 5, 2] would pass the check? If so one could use isSorted (https://nim-lang.github.io/Nim/algorithm.html#isSorted%2CopenArray%5BT%5D)

impulse/lfsr.nim Outdated
## - taps: Feedback polynomial taps as a Tensor[int] of exponents in
## descending order. Note that exponent 0 can be omitted, since it is
## always implicitly added. For example, to use the `x^5 + x^3 + 1`
## polinomial you can set taps to `[5, 3].toTensor` or to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## polinomial you can set taps to `[5, 3].toTensor` or to
## polynomial you can set taps to `[5, 3].toTensor` or to

impulse/lfsr.nim Outdated
self.count += 1
self.outbit = result

return result
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really needed (the python heritage shines through 🤣)

@AngelEzquerra AngelEzquerra force-pushed the lfsr branch 2 times, most recently from a7a05e6 to 9f859da Compare March 26, 2024 13:09
impulse/lfsr.nim Outdated Show resolved Hide resolved
Improve handling of seqs.

Co-authored-by: Vindaar <[email protected]>
impulse/lfsr.nim Outdated
Comment on lines 171 to 182
iterator generator*(lfsr: var LFSR,n: int,
store_sequence: static bool = false): bool =
## Generator that will generate a random sequence of length `n`
##
## Inputs:
## - Preconfigured LFSR object
## - n: Number of random values to generate
## - store_sequence: static bool that enables saving the generated sequence
##
## Return:
## - Generated boolean values
yield lfsr.next(store_sequence = store_sequence)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand why this is an iterator. The iterator only returns a single element. So it might as well just be a procedure? Calling it in a for loop will be redundant, no?

And it doesn't loop over n either so that next is called n times. Oversight or intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed the iterator to items, fixed the problem you explained and added a couple of new arguments to make it more flexible. Those new options might be overkill though :-). If you feel they are too much please let me know and I'll remove them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@Vindaar
Copy link
Member

Vindaar commented Mar 30, 2024

Can't much comment on the logic in next, given my lack of LFSR knowledge. Otherwise it looks great!

@Vindaar Vindaar merged commit 3faacda into SciNim:master Apr 15, 2024
6 checks passed
@AngelEzquerra AngelEzquerra deleted the lfsr branch April 20, 2024 09:36
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.

2 participants