bugGNU Octave - Bugs: bug #59850, uniquetol missing, so I...

 
 

bug #59850: uniquetol missing, so I implemented it

Submitted by:  None
Submitted on:  Sun 10 Jan 2021 12:10:34 PM UTC  
 
Category:  Octave Function Severity:  1 - Wish
Priority:  3 - Low Item Group:  Feature Request
Status:  Ready For Test Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  Open Release:  dev
Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 21 Jan 2021 05:59:25 PM UTC, comment #6: 

I received your user details privately.

I didn't see the argument "row" mentioned in Matlab's documentation. I also couldn't get Matlab to accept that argument. Removing it also allowed to simplify (and complete) the input check.

I also wasn't able to trigger the error in line 121. Can it be removed?

Anyway, I pushed the new function here:
https://hg.savannah.gnu.org/hgweb/octave/rev/f3272029d42c

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Mon 18 Jan 2021 06:17:26 PM UTC, comment #5: 

Sorry, I reloaded the page and forgot to attach the file again.

I understand that you'd prefer to not post your email address as plain text in the comments.
Could you download the attached patch, exchange the line starting with "# User" with your name and email address, and re-upload the patch?
I could add your name to the list of contributors in the manual after that. Or do you prefer to not be added to that list?

(file #50746)

Markus Mützel <mmuetzel>
Project Member
Mon 18 Jan 2021 06:13:21 AM UTC, comment #4: 

I fixed the problematic sentence. I also fixed a few typos. I changed the numbers and some of the function calls log10 to log etc to be different from the Matlab documentation but we need to test the same behavior as the function does almost the same thing.

I also changed the function to call unique if the input was not numeric with double or single precision, and issued a warning if it does this.

I did not see another file uploaded by you so I modified my copy.

I can provide contact details but I don't want to give them on a public webpage. How do I go about this?

(file #50736)

Anonymous
Sun 17 Jan 2021 05:07:32 PM UTC, comment #3: 

Thank you very much. That looks very good to me!

All tests pass for me.

I made only some minor changes to the docstring and style (see attachment).

Some remarks:

  • I don't understand the last sentence before the example in the docstring. (I'm no native speaker. So that might be the reason.) Could you please re-phrase that sentence? Maybe split it up in two or more sentences.
  • It looks like some of the BISTs you added are copies from the examples in Matlab's documentation. While this is a good check to see if the function is compatible, I'm not sure if we can take those and distribute them under GPL. Could you please remove those or replace them with other tests?

I didn't check yet if the docstring compiles. But I think that this function is almost ready to be added.

Would you like to appear as the author in the changeset? We usually use a real first (and last) name together with a valid email address as the author of a changeset.
Which name and email address could I use?
Would you like to appear in the list of contributors (that appears at the beginning of the manual)?

If you prefer to remain anonymous, I could also push the change with my name.

Markus Mützel <mmuetzel>
Project Member
Sun 17 Jan 2021 12:51:33 PM UTC, comment #2: 

I have attached the new version, following the code style, with documentation and tests. I copied from unique extensively. There are a few differences between unique and uniquetol. For example, inf does not make sense to combine into a single inf as the difference between two inf values could be anything so I treat it like nan. I do not consider data types other than double and single.

(file #50729)

Anonymous
Thu 14 Jan 2021 12:02:16 PM UTC, comment #1: 

Thank you for your contribution.

I haven't tested yet.
But it would help a lot if you could add some documentation (texinfo). Please, take a look at some other .m file that is part of Octave at how that could look like.
Also please add some built-in self test (see e.g. %!test sections in other files) and maybe also some %!demo-s.
Ideally, please provide a patch that adds that file to the default branch of Octave's Mercurial repository. But if you aren't set up for that, the .m file will also do. (Please don't paste the function as plain text in a comment.)
In that file, please add the GPL license that you can find at the top of other .m files as well. That tells us that you are willing (and able) to distribute the function under that license.
Also, please try and follow Octave's coding style:
https://wiki.octave.org/Octave_style_guide

I sincerely hope this doesn't intimidate in any way. We are always happy to see new contributors. So, please don't hesitate to ask if anything is unclear.

Markus Mützel <mmuetzel>
Project Member
Sun 10 Jan 2021 12:10:34 PM UTC, original submission:  

I get the message uniquetol is missing from Octave and I could not find it in a package so I have implemented it. I do not have Matlab so I can not directly compare it but I have run all of the examples on the Matlab uniquetol help page and I received the same output from my implementation as shown on that page.

The Implementation

function [C,IA,IC]=uniquetol(A,varargin)


maxabsA=max(abs(A(:)));
ByRows=false;
OutputAllIndices=false;
DataScale=maxabsA;
sizeA=size(A);
if nargin()==1||~(isnumeric(varargin{1})&&isscalar(varargin{1}))
  if isa(A,'double')
    tol=1e-12;
  elseif isa(A,'single')
    tol=1e-6;
  endif
else
  tol=varargin{1};
end

for k=1:length(varargin)
  if isnumeric(varargin{k})
    continue
  elseif ischar(varargin{k})
    if strcmpi(varargin{k},'rows')
      ByRows=true;
    elseif strcmpi(varargin{k},'byrows')
      ByRows=logical(varargin{k+1});
    elseif strcmpi(varargin{k},'OutputAllIndices')
      OutputAllIndices=logical(varargin{k+1});
    elseif strcmpi(varargin{k},'DataScale')
      DataScale=varargin{k+1}(:).';
      columnsDataScale=columns(DataScale);
      if ~(columnsDataScale==1||columnsDataScale==sizeA(2))
        error("Incorrect size of data scale.");
      endif
    endif
  endif
end
if ~ByRows
  A=A(:);
end
x=A;
points=rows(x);
d=columns(x);
Iall=zeros(points,1);
I=nan(d,1);
IA={};
J=nan(d,1);
j=1;
ii=0;
tolDataScale=tol*DataScale;
for i=1:points
  if any(Iall==i)
    continue
  else
    eq=all(abs(x-x(i,:))<tolDataScale,2);
    sumeq=sum(eq);
    ia=find(eq);
    if OutputAllIndices
      IA{end+1}=ia;
    endif
    Iall(ii+(1:sumeq))=ia;
    I(j)=ia(1);
    J(eq)=j;
    ii+=sumeq;
    j+=1;
  end
end
C=x(I,:);
if sizeA(1)==1&&sizeA(2)>1
  C=C.';
end
if ~OutputAllIndices
  IA=I(1:j-1);
endif
IC=J;
end

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #50736:  uniquetol.m added by None (8KiB - text/plain)
file #50729:  uniquetol.m added by None (7KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    17:59 mmuetzel StatusPatch Submitted => Ready For Test
    2021-01-18 mmuetzel Attached File- => Added bug59850_uniquetol.patch, #50746
        StatusIn Progress => Patch Submitted
    2021-01-18 None Attached File- => Added uniquetol.m, #50736
    2021-01-17 mmuetzel StatusNone => In Progress
    2021-01-17 None Attached File- => Added uniquetol.m, #50729
    2021-01-11 jwe Severity3 - Normal => 1 - Wish
        Priority5 - Normal => 3 - Low
        Item GroupMatlab Compatibility => Feature Request
        Release6.1.0 => dev

    Back to the top


    Powered by Savane 3.5