Re: [Discuss-gnuradio] Feature #394

[Andrew Davis]

>As for the complex bandpass, we really do need that. I know I've used it in 
>GNU Radio-based projects before, and I'd bet >others have/are, too. Creating 
>it as an LPF and the frequency shifting it is a perfectly valid way of 
>creating this kind of a >filter, and it enforces symmetrical transition bands.

OK, I have the code done It just uses different arguments, Instead of
giving ignored lower transition band starts and stops, it just asks
for the desired center. ( If again no one objects )

>If there's any way we can fix those bugs in the Python code, first, I'd prefer 
>to do that.

If I can remember what they are, most problems didn't change the
output though ( like unused, redundant, or improper code ).

~Andrew

On Thu, Mar 8, 2012 at 1:32 PM, Tom Rondeau <address@hidden> wrote:
> On Thu, Mar 8, 2012 at 1:07 PM, Andrew Davis <address@hidden>
> wrote:
>>
>> Thanks,
>>
>> All the 'fix' comments are there as I was debugging on a remote
>> machine using github as an intermediate for all my little changes.
>> I'll get those merged into real comments before the real merge. Same
>> for the AM demods, my github optfir branch is for my local testing and
>> will only include optfir core in the merge to my master repo. I think
>> I'll get the QA done over spring break ( next week ) along with more
>> of blks2.
>
>
> Ah, of course. I understand that mode of working. Not ideal, but sometimes
> necessary. I mistook the state of your branch.
>
>>
>> Which brings me to a few questions, complex_band_pass appears to just
>> be a complex shifted low pass and therefor does not actually do what
>> it says it does ( the first band edge is only used to find the center
>> frequency and not shape the actual band edge ), it is also not used
>> anywhere ( except the filter design demo ), would anyone object if it
>> was not included in the C++ version? Also I found a few small bugs in
>> porting ( for instance 'desired_ampls = (0, 1)' I believe should have
>> been 'desired_ampls = (0, gain)' ) so my version might not produce the
>> exact same results in all cases  but produce hopefully more correct
>> taps.
>>
>> I figured float to be enough for any practical use, just a little
>> venting in comments :)
>>
>> Now if only the FSF would get back with me......
>>
>> ~Andrew
>
>
> If there's any way we can fix those bugs in the Python code, first, I'd
> prefer to do that. It'd be nice to make sure both worlds agree.
>
> As for the complex bandpass, we really do need that. I know I've used it in
> GNU Radio-based projects before, and I'd bet others have/are, too. Creating
> it as an LPF and the frequency shifting it is a perfectly valid way of
> creating this kind of a filter, and it enforces symmetrical transition
> bands. It might actually be nice to have a complex bandpass that fully
> utilizes the PM algorithm to set the transition bands as desired on either
> ends, but that would only be useful for a special cases.
>
> Thanks,
> Tom
>
>
>
>>
>> On Thu, Mar 8, 2012 at 11:51 AM, Tom Rondeau <address@hidden> wrote:
>> > Hi Andrew,
>> >
>> > Thanks again for taking the time to look into this. I have a few
>> > comments.
>> >
>> > I was looking over your optfir branch and it needs a bit of work. Commit
>> > messages like 'fix' aren't helpful to the log and make figuring out what
>> > you
>> > did confusing. Can you go through and squash these into single commits
>> > with
>> > a more helpful commit message (take a look at our commit messages on
>> > master
>> > for what we're looking for).
>> >
>> > Also, it would be nice to only have this branch handle the optfir
>> > conversion
>> > and not have the AM demodulators in there as well. That can confuse
>> > what's
>> > going one, and it's easier to look at a branch merge that focuses on
>> > doing
>> > one thing.
>> >
>> > Is there any way that we can verify the results of the code? We don't
>> > have
>> > any QA code for the filter generators, but it should be easy enough to
>> > make
>> > a set of known taps using optfir.py as our benchmark and then use the
>> > same
>> > parameters to see what the results from the C++ implementation are.
>> > That'd
>> > be a nice test so we can verify and prove that the new code works well.
>> >
>> > I also noticed your comments on losing precision by casting to float. I
>> > wouldn't worry about that kind of precision since any amount of noise is
>> > going to ruin that for you, anyway. 32-bit floats is really more than
>> > enough
>> > for anything but the most extreme scientific tasks, and floats work a
>> > hell
>> > of a lot faster on almost any processor.
>> >
>> > Thanks again,
>> > Tom
>> >
>> >
>> > On Mon, Feb 27, 2012 at 1:11 PM, Tom Rondeau <address@hidden> wrote:
>> >>
>> >> On Mon, Feb 27, 2012 at 1:01 PM, Andrew Davis <address@hidden>
>> >> wrote:
>> >>>
>> >>> >[Did you copy any files or text written by someone else in these
>> >>> > changes? Even if that material is free software, we need to know
>> >>> > about it.]
>> >>>
>> >>> Is this needed if I only copied from GNUradio, and if so, how would I
>> >>> reference optfir.py and firdes, by name or author? Or for the request
>> >>> do I only need to say yes?
>> >>>
>> >>> Thanks
>> >>
>> >>
>> >> I'm never sure on these things. But they want this for their records
>> >> and
>> >> it doesn't obligate you to anything. Just write down something like
>> >> "Yes,
>> >> optfir.py and gr_firdes.cc from GNU Radio." That should be enough info
>> >> for
>> >> them.
>> >>
>> >> Thanks,
>> >> Tom
>> >>
>> >>
>> >>
>> >>>
>> >>> On Mon, Feb 27, 2012 at 11:11 AM, Tom Rondeau <address@hidden>
>> >>> wrote:
>> >>> > Hi Andrew,
>> >>> > Now that you're contributing your work to us, we have to work out
>> >>> > the
>> >>> > annoying bureaucratic paperwork. All GNU Radio code gets its
>> >>> > copyright
>> >>> > assigned to the FSF, and so we need an agreement with you for this.
>> >>> > I've
>> >>> > attached the 'conditions.text' file to explain this to you more.
>> >>> >
>> >>> > Depending on how you want to go about this, you can either fill out
>> >>> > the
>> >>> > 'request-assign.changes' to cover these changes, or the
>> >>> > 'request-assign.future' that will set you up for any future
>> >>> > contributions to
>> >>> > GNU Radio, as well. If you anticipate contributing work in the long
>> >>> > run, the
>> >>> > latter would be the best option for all of us, even if it does take
>> >>> > a
>> >>> > few
>> >>> > weeks to turn it around.
>> >>> >
>> >>> > Thanks!
>> >>> > Tom
>> >>> >
>> >>> >
>> >>> >
>> >>> >
>> >>> > On Mon, Feb 27, 2012 at 11:03 AM, Tom Rondeau <address@hidden>
>> >>> > wrote:
>> >>> >>
>> >>> >> On Fri, Feb 24, 2012 at 9:48 PM, Andrew Davis
>> >>> >> <address@hidden>
>> >>> >> wrote:
>> >>> >>>
>> >>> >>> Ok, I've got a branch on github:
>> >>> >>> https://github.com/glneo/gnuradio-davisaf/tree/optfir with optfir
>> >>> >>> ported and working in C++, it's part of gr ( gr.optfir ) like
>> >>> >>> firdes.
>> >>> >>> This keeps the filter design tools together and allows the old
>> >>> >>> optfir
>> >>> >>> to still be imported and used until I can get all the examples
>> >>> >>> ported
>> >>> >>> ( which will just be changing "optfir" to "gr.optfir" )
>> >>> >>>
>> >>> >>> This is kinda just an update, it will probably not be ready for
>> >>> >>> merging until I finish porting the blks2 hier to C++. Then all the
>> >>> >>> examples can be done in both python and C++, hopefully this opens
>> >>> >>> up
>> >>> >>> the API a bit.
>> >>> >>
>> >>> >>
>> >>> >> Andrew,
>> >>> >> Thanks. It might be easier to handle these in stages from a merge
>> >>> >> standpoint. Since you're just adding stuff, it's easy to add bits
>> >>> >> and
>> >>> >> pieces. If the optfir is ready, we can look into merging it while
>> >>> >> you
>> >>> >> make
>> >>> >> another branch for other conversions to C++.
>> >>> >>
>> >>> >> Thanks!
>> >>> >> Tom
>> >>> >>
>> >>> >>
>> >>> >>>
>> >>> >>> On Wed, Feb 8, 2012 at 11:27 PM, Andrew Davis
>> >>> >>> <address@hidden>
>> >>> >>> wrote:
>> >>> >>> > Thanks, I think i'll work on QA too while i'm at it then.
>> >>> >>> >
>> >>> >>> >
>> >>> >>> > On Wed, Feb 8, 2012 at 10:32 PM, Tom Rondeau <address@hidden>
>> >>> >>> > wrote:
>> >>> >>> >>
>> >>> >>> >> On Tue, Feb 7, 2012 at 9:52 PM, Andrew Davis
>> >>> >>> >> <address@hidden>
>> >>> >>> >> wrote:
>> >>> >>> >>>
>> >>> >>> >>> Hello all,
>> >>> >>> >>>
>> >>> >>> >>> I would like to help expand the C++ API, so I'm attempting to
>> >>> >>> >>> work on
>> >>> >>> >>> Feature #394 or "Re-implement hierarchical blocks currently
>> >>> >>> >>> living in
>> >>> >>> >>> blks2
>> >>> >>> >>> in C++ and put into gnuradio-core/src/lib/hier." I've started
>> >>> >>> >>> on
>> >>> >>> >>> am_demod.py
>> >>> >>> >>> but this requires optfir, which is also in python, I think
>> >>> >>> >>> this
>> >>> >>> >>> should also
>> >>> >>> >>> be available to us C++ users, so i'm converting it too.  I'm
>> >>> >>> >>> new
>> >>> >>> >>> to
>> >>> >>> >>> GnuRadio
>> >>> >>> >>> and would like to know if i'm on the right track before I get
>> >>> >>> >>> to
>> >>> >>> >>> far.
>> >>> >>> >>> The
>> >>> >>> >>> files so far are included.
>> >>> >>> >>>
>> >>> >>> >>> Thank you.
>> >>> >>> >>
>> >>> >>> >>
>> >>> >>> >>
>> >>> >>> >> Hi Andrew,
>> >>> >>> >> It looks to me like you're on the right track! Thanks for
>> >>> >>> >> making a
>> >>> >>> >> go
>> >>> >>> >> at
>> >>> >>> >> it.
>> >>> >>> >>
>> >>> >>> >> So you seem to have the general style correct in the files that
>> >>> >>> >> I
>> >>> >>> >> looked
>> >>> >>> >> at. Once it's coded, the ultimate test is, obviously, to make
>> >>> >>> >> sure
>> >>> >>> >> that the
>> >>> >>> >> data produced by any of these guys is the same as is produced
>> >>> >>> >> by
>> >>> >>> >> the
>> >>> >>> >> Python
>> >>> >>> >> versions. This is a good case where the QA code would be
>> >>> >>> >> useful,
>> >>> >>> >> so we
>> >>> >>> >> would
>> >>> >>> >> have a set of tests with known output that you could compare
>> >>> >>> >> against
>> >>> >>> >> the new
>> >>> >>> >> implementations. Unfortunately, I don't see an QA for the
>> >>> >>> >> optfir,
>> >>> >>> >> but
>> >>> >>> >> it
>> >>> >>> >> would probably be good to have one.
>> >>> >>> >>
>> >>> >>> >> Tom
>> >>> >>> >>
>> >>> >>> >
>> >>> >>
>> >>> >>
>> >>> >
>> >>
>> >>
>> >
>
>