bugGNU Octave - Bugs: bug #58832, [octave forge] (image) No BISTs in...

 
 

bug #58832: [octave forge] (image) No BISTs in iradon

Submitter:  Avinoam Kalma <avinoam>
Submitted:  Sun 26 Jul 2020 08:37:18 PM UTC
   
 
Category:  Octave Package Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  avinoam
Originator Name:  Avinoam Open/Closed:  * Closed
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 15 Jan 2021 10:11:35 PM UTC, comment #8: 

I have now pushed these additional unit tests (V2 patch with some additional whitespace changes, see comment #7) to the image repository: http://hg.code.sf.net/p/octave/image/rev/8afbbce94a9f

I think this issue is done, now.

Hartmut <hardy>
Tue 29 Dec 2020 07:29:50 PM UTC, comment #7: 

Thanks for opening a new bug report for the output values of iradon, Avinoam.

Since this new report might cause some code changes to iradon (sooner or later), I decided to generate some more tests for iradon to avoid regressions. I attach a PATCH that

  • adds one test with the fully compatible output of iradon with the "none" filter, mentioned by Avinoam in comment #2.
  • adds one long new test, that calls iradon with every single filter option and compares the output to the Matlab result. This test should still pass when we improve Matlab compatibility, but it should fail if the compatibility gets worse.


Please review those new tests in the patch file (V2).

(file #50587)

Hartmut <hardy>
Mon 28 Dec 2020 09:18:00 PM UTC, comment #6: 

Thanks for the fix @Hartmut.

I have opened a new bug report (bug #59773) regarding comment #2.

closing.

Avinoam Kalma <avinoam>
Group Member
Tue 22 Dec 2020 05:00:49 PM UTC, comment #5: 

I have now pushed the patch from comment #1 with some first BISTs for iradon.m to the image repo: http://hg.code.sf.net/p/octave/image/rev/f04eb72099da

I think we should then close this issue as FIXED.

@Avinoam: Yes, please open a new bug report about the compatibility of our iradon function. You observation (already compatible output values for "none" option) gives me the necessary amount of hope that it might be possible to turn our iradon function even more Matlab compatible than today.

Hartmut <hardy>
Tue 22 Dec 2020 05:46:43 AM UTC, comment #4: 

@Hartmut: of course there is no objection. Please push the new tests. Thanks :-)

Note that if the filter type is "none", the results are the same as Matlab results, but if there are filters, like the default "Ram-Lak", the results are differ. It seems that the implementation of the filters are wrong.
Should we open a new bug report, or continue here to discuss it?

Avinoam Kalma <avinoam>
Group Member
Mon 21 Dec 2020 07:37:10 PM UTC, comment #3: 

Thanks for reviewing this, Avinoam.

Yes, I know that in general the results of Octave's iradon function are not 100% the same as from Matlab. But I seems that "our" results from the Octave version are nevertheless of the "same quality" as the Matlab function results.

So I decided to design tests (BISTs) that make sure, that the current quality of our results cannot degrade without us noticing it. The new tests are also designed such that they will still pass if our Matlab compatibility improves sometime in the future.

If there are no objections, I would then push those new tests to the image repo during the next days.

Hartmut <hardy>
Mon 21 Dec 2020 05:10:25 PM UTC, comment #2: 

Thanks @Hartmut for the tests. They all passed.

Note there is incompatibility between Matlab and Octave:

Ocatve:


>> iradon(radon(ones(2,2),0:5),0:5, 'nearest', 'none')
ans =

   0.4264   2.7859   2.7152   0.3557
   0.4264   2.7859   2.7152   0.3557
   0.4264   2.7859   2.7152   0.3557
   0.4264   2.7859   2.7152   0.3557


Matlab:


>> iradon(radon(ones(2,2),0:5),0:5, 'nearest', 'none')

ans =

    0.4264    2.7859    2.7152    0.3557
    0.4264    2.7859    2.7152    0.3557
    0.4264    2.7859    2.7152    0.3557
    0.4264    2.7859    2.7152    0.3557


But:

Octave:


>> iradon(radon(ones(2,2),0:5),0:5, 'nearest')
ans =

  -0.3677   0.7481   0.7127  -0.3903
  -0.3677   0.7481   0.7127  -0.3903
  -0.3677   0.7481   0.7127  -0.3903
  -0.3677   0.7481   0.7127  -0.3903


Matlab:


>> iradon(radon(ones(2,2),0:5),0:5, 'nearest')

ans =

   -0.3593    0.7563    0.7210   -0.3820
   -0.3593    0.7563    0.7210   -0.3820
   -0.3593    0.7563    0.7210   -0.3820
   -0.3593    0.7563    0.7210   -0.3820


Avinoam Kalma <avinoam>
Group Member
Sat 19 Dec 2020 10:14:35 PM UTC, comment #1: 

I have attached a PATCH file that adds some first BIST tests to iradon.m. Please review.

(file #50526)

Hartmut <hardy>
Sun 26 Jul 2020 08:37:18 PM UTC, original submission:  


As @Hartmut commented in bug #58567, there are no BISTs in iradon function.

Avinoam Kalma <avinoam>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50587:  iradon_moretests_V2.patch added by hardy (3KiB - text/x-patch)
file #50526:  iradon_tests_V1.patch added by hardy (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by hardy (Updated the item)
  • -email is unavailable- added by avinoam (Submitted the item)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by avinoam
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-12-29 hardy Attached File- Added iradon_moretests_V2.patch, #50587
    2020-12-28 avinoam StatusNone Fixed
        Open/ClosedOpen Closed
    2020-12-19 hardy Attached File- Added iradon_tests_V1.patch, #50526
    2020-07-26 avinoam Carbon-Copy- Added carandraug
        Carbon-Copy- Added hardy

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code