Implements 'Keep' Shifting Config

[Jonah-Foley]

What This PR Achieves

After reviewing the fxpmath source, I saw that there was no current implementation for the 'Keep' shifting configuration, despite it being a valid option when constructing an Fxp object. This is important to me for the purpose of hardware modelling as i'd like a fixed-point type which:

• Retains a fixed size upon being shifted
• Discards bits which are shifted out of either end
• Correctly differentiates between arithmetic and logic shifts for signed numbers.

Implementation Details

This PR implements that. The basic approach is to shift the raw value (as in the case of the 'expand' approach) however then bitmask the result with a mask of the form 01...1 where the number of 1's is equal to the input word size. We then create a new Fxp object from the masked value and return.

Note, when we construct the new value, we have to explicitly do so from a binary string with the formatting

f"0b{(shift & mask):0{n_word}b}"

This is necessary for two cases:

1. For a shift operation which leads to a value of 0b1 (i.e.1 << 0), this must be treated as 0b00..01 when calling set_val(), otherwise it will be treated as Fxp('0b1', True, N, 0) = 0b11..11
2. For a shift operation which puts a 1 into the MSB of a signed number, this should be treated with the correct signedness i.e.
   0b0100 << 1 == 0b1000 == -8. Without this formatting 0b0100 << 1 is treated as 8, which would not fit into a signed 4-bit value, and hence passing it to set_val() would cause the value to saturate and the resulting object would take on the value 0b0111 == 7

Tests

I have written some tests in test_operators.py to demonstrate the functionality. I anticipate that there may be a better way to achieve this behaviour without use of the string formatting, so I am very open to suggestions before merging.

[Jonah-Foley]

@francof2a Do you have any thoughts?

[francof2a]

Hello @Jonah-Foley
Thanks for the PR to solve this issue.

I do some previous comments before review your PR implementation:

• The actual version is 0.4.9 (just taken out of the oven), so you should merge into dev-0.4.10 branch. Please update your fork from last release.
• did you test this implementation with numpy arrays with more than one element? I have not see those cases in your tests.

I will be reviewing your implementation these days!

[Jonah-Foley]

I have:

• Modified the base branch to be dev-0.4.10 .
• Implemented a test case to test the 'Keep' shifting operation for array values.
• Subsequently modified the implementation when I realised my old implementation didnt work for arrays.

I understand the testing could be more rigorous however I believe the current test framework doesn't really lend itself to that. I understand it is a goal of yours to change this in the future. Let me know what you think. Still open to modifications.

let me know what you think @francof2a

[francof2a]

I see two issues with your solution:

1. It doesn't support N-Dimensional arrays, where N > 1. You're assuming that array is a simple list. You could check self.val.ndim in order to check the dimensions but then your algorithm should iterate across them and it could be difficult and inefficient.
2. The value for the new Fxp is set using a binary string. This way will take longer because integer (raw) value must be interpreted from binary string. Then, if you have a large list or array, this will take a lot of time.

Thinking about the problem you found, I think a good solution would be shift the value and keep the n_words bits; but if the Fxp is signed and the shifted value now is negative (MSb = 1), we must calculate the two's complement of the shifted value.

The __lshift__ function is then:

def __lshift__(self, n):
    if self.config.shifting == 'expand':
        n_word = max(self.n_word, int(np.max(np.ceil(np.log2(np.abs(self.val)+0.5)))) + self.signed + n)
        new_value = self.val << np.array(n, dtype=self.val.dtype)
    elif self.config.shifting == "keep":
        n_word = self.n_word

        @utils.array_support
        def _raw_lshift(val, nshift, nbits, signed):
            # shift and keep only nbits after shifting
            val = (val << np.array(nshift, dtype=val.dtype)) % (1 << nbits) 

            # if most significant bit is 1 and signed take two's complement
            if signed and (int(val) & (1 << (nbits - 1))) != 0:
                val = val - (1 << nbits)
            return val

        new_value = _raw_lshift(self.val, nshift=n, nbits=n_word, signed=self.signed)
    else:
        n_word = self.n_word
        new_value = self.val << np.array(n, dtype=self.val.dtype)

    y = Fxp(None, signed=self.signed, n_word=n_word, n_frac=self.n_frac)
    y.set_val(new_value, raw=True, vdtype=self.vdtype)   # set raw val shifted
    
    return y
    

Note that I used @utils.array_support that is a decorator to vectorize the function, what implements the N-Dimensional array support.

I added extra tests in order to check the N-D support and for -1 value left-shifted in signed Fxp. Your whole test section updated is:

# keep shift
## left unsigned
x = Fxp(1, False, 8, 0, shifting="keep")

prev = x.get_val()
for i in range(x.n_word):
    assert (x << i)() == prev
    prev = 2 * prev
assert (x << 8)() == 0

## right unsigned
x.set_val(128)
prev = 128
for i in range(x.n_word):
    assert (x >> i)() == prev
    prev = prev // 2
assert (x >> 8)() == 0

## left signed
x = Fxp(1, True, 8, 0, shifting="keep")
    
prev = 1
for i in range(x.n_word - 1):
    assert (x << i)() == prev, f"{i}, {prev}"
    prev = 2 * prev
assert (x << x.n_word - 1)() == -128
assert (x << x.n_word)() == 0

x.set_val(113)
for i in range(x.n_word):
    assert (x << i).bin() == x.bin()[i:] + (i * "0")

x.set_val(-1)
for i in range(x.n_word):
    assert (x << i).bin() == (x.n_word - i) * '1'  + (i * "0")

## right signed
x.set_val(-128)
for i in range(x.n_word):
    assert (x >> i).bin() == (i + 1) * "1" + (x.n_word - (i + 1)
assert (x >> 8).bin() == x.n_word * "1"

## None type
x = Fxp(None, False, 8, 0, shifting="keep")
for i in range(10):
    assert x << i == 0

## arrays
x = Fxp([1, 2, 4, 8], False, 8, 0, shifting="keep")
assert np.all(x << 1 == x*2)

x.set_val([128])
assert np.all(x << 1 == 0)

x = Fxp([[1,2],[4,8]], False, 8, 0, shifting="keep")
assert np.all(x << 1 == x*2)

Please let me know if it's right for you.

[Jonah-Foley]

Awesome, thank you for your implementation. I shall review it in the coming few days.