TorchDSP / torchsig

TorchSig is an open-source signal processing machine learning toolkit based on the PyTorch data handling pipeline.
MIT License
174 stars 40 forks source link

DescToBBoxSignalDict creates incorrect box. #228

Closed jrounds closed 2 months ago

jrounds commented 1 year ago

First thanks for the paper and the models.

The bug I am going to describe slipped into a couple spots in target_transforms.py.

Describe the bug DescToBBoxSignalDict creates incorrect box.

To Reproduce

Latest main branch

Suppose you have a label loaded like this (which I got from your code examples):

>>> label[0]
<torchsig.utils.types.SignalDescription object at 0x7f2ca0624fa0>
>>> label[0].__dict__
{'sample_rate': None, 'num_iq_samples': None, 'lower_frequency': -0.01416015625, 'upper_frequency': 0.032562255859375, 'bandwidth': 0.04672, 'center_frequency': 0.0092, 'start': 0.1271, 'stop': 0.2913, 'duration': 0.1642, 'snr': 30.95, 'bits_per_symbol': 0, 'samples_per_symbol': 0.0, 'excess_bandwidth': 0.0, 'class_name': '8psk', 'class_index': None}

We expect some sort of box out of this

DescToBBoxSignalDict()(label[0])

But we get this:

{'labels': tensor([0]), 'boxes': tensor([[0.2092, 0.2092, 0.2092, 0.2092]])}`

Strange 0 width point box.

Why?

How about this: https://github.com/TorchDSP/torchsig/blob/main/torchsig/transforms/target_transforms.py#L1116

The bbox starts as an array of length 4 but that [0] takes out the first element. And then for the assignment, since the array already has a shape of (...,4) ,the assignment just duplicates the [0] value across the box. Result: no box.

There is a blame for it: image

It looks a little more correct to me before that commit v0.4.0 (https://github.com/TorchDSP/torchsig/pull/137[)](https://github.com/TorchDSP/torchsig/commit/c9b1193fe8aab02d0b5e3478ddc94b57bf4c69bb) which added the [0] indexing of the 4 tuple.

smthzch commented 8 months ago

I ran into this issue too. It is also present in DescToBBoxDict: https://github.com/TorchDSP/torchsig/blob/53cb06343cb89ff1e764c6813fe1d71d981cae0f/torchsig/transforms/target_transforms.py#L1068

slightY commented 7 months ago

I'm having the same problem, and I think the right thing to do would be to delete the [0] from line 1068 of the code above

MattCarrickPL commented 2 months ago

Large transform and bounding box rework since 0.4.2, so closing out.