Skip to content

[Bioc-devel] Noticed some XDouble* functionality missing in IRanges

6 messages · Michael Lawrence, Steve Lianoglou, Hervé Pagès

Original
Steve Lianoglou
# 
Greetings,

I tried to slice a "normal" numeric/double vector and was a bit
surprised to get an error.

I see some support for doing this baked in, eg. the
src/XDoubleViews_utils.c :: XDouble_slice function, but it's not
exposed to the useR yet.

Would anybody mind if I start building up support for XDouble's to be
more inline with the support available in XIntegers? I'm just planning
to add XDoubleView-class.R and XDoubleViews-utils.R files that
essentially mimic their XIntegerView sister files.

Out of curiosity, was there a reason this wasn't done other than no
one had to scratch this itch yet? Just want to make sure I'm not
missing something obvious?

Thanks,
-steve
Steve Lianoglou
# 
On Thu, Jun 9, 2011 at 1:16 PM, Michael Lawrence
<lawrence.michael at gene.com> wrote:
Well, I've got at least one of those, so ... will do.

-steve
Hervé Pagès
#
On 11-06-09 10:59 AM, Steve Lianoglou wrote:
Sounds good Steve. Yes please go ahead.

H.

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Steve Lianoglou
    

    

      
      #
    

  


  

      
Hi,

2011/6/9 Herv? Pag?s <hpages at fhcrc.org>:

            

      
      
      
    

        
I whipped this up and committed into IRanges revision 56093.

I essentially took all of the R and C code for XInteverView stuff and
ported it to work over XDouble's. All seems to work as expected and I
put basic unit tests into  inst/unitTests/test_XDoubleViews.R

A few comments for feedback:

(1) One point that's different in the XDouble is inside the
XDoubleViews-class::XDoubleViews.equal function. The result
(TRUE/FALSE) value saved in each ans[i] is wrapped in a call to
`all()` since X{Double|Integer}Views.view1_equal_view2 returns a
logical vector which is as long as the elements that are passed into
that function.

If it's not wrapped in `all()` like it currently is in
XIntegerViews.equal, then you get a warning that you are trying to
assign a logical vector to 1 position (in `ans`) and only the first
value is taken -- which I think is wrong. So, I think this change
should be applied to the same position in the `XIntegerViews.equal`
function, but I didn't do it since I thought I might be missing
something.

(2) The XDoubleViews_viewSums function in XDoubleViews_utils.c is
different from its sister XInteger function in that it's currently not
checking for an overflow in the accumulated sum(s). My C is a bit
rusty and I wasn't sure how to correctly do so here ... could someone
please tweak and fix? It's on line 184-187 XDoubleViews_utils.cf .

Thanks,
-steve

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Hervé Pagès
    

    

      
      #
    

  


  

      
Hi Steve,

        
On 11-06-09 08:48 PM, Steve Lianoglou wrote:

            

      
      
      
    

        
Thanks for this!

            

      
      
      
    

        
Good catch. I fixed XIntegerViews.equal().

            

      
      
      
    

        
Note that the check for overflow in XIntegerViews_viewSums() is broken.

My understanding is that when summing double values there is no need
to check for an overflow because the built-in arithmetic for doubles
will produce an inf (which is a special double value), and operations
involving an inf are well defined.

I checked your new code and discovered that we had some long-standing
issues with the pre-existing XIntegerViews code that you mimicked.
I fixed them but overall I think there is still lot of room for
improving the XIntegerViews/XDoubleViews code (I've added comments
where I saw possible improvements). In particular more code should
be shared, especially at the C level.

Thanks again for your contribution,
H.