discuss-gnuradio
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Discuss-gnuradio] Feature #394


From: Tom Rondeau
Subject: Re: [Discuss-gnuradio] Feature #394
Date: Thu, 8 Mar 2012 17:56:54 -0500

On Thu, Mar 8, 2012 at 5:40 PM, Andrew Davis <address@hidden> wrote:
>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 )

So one thing to keep in mind is how we deal with changes in the API of the code. Any time there is a change to an interface or arguments list (unless it's a new argument with a default value at the end of the the list), it's an API change, which means that it has to go into the 'next' branch.

For this, since we're talking about removing the Python code and replacing it with the C++ versions, it's probably safer to just say that this will go into 'nex' and be a part of the 3.6 release.

 
>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

If they didn't effect the output, no problem, but since it might slightly here or there, it's just another reason to think about doing this on the 'next' branch.

This probably means that you don't have to do anything more, and I'm assuming that you branched this off master initially. Since next is a superset of master, the merge should be pretty easy and clean (conflicts with autotools files, but that's expected). I'm just telling you this so you understand how we thing about these things and making changes to the code.

Thanks,
Tom

 
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
>> >>> >>> >>
>> >>> >>> >
>> >>> >>
>> >>> >>
>> >>> >
>> >>
>> >>
>> >
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]