Test Results

37 tests ±0 37 :heavy_check_mark: ±0 0s :stopwatch: ±0s   2 suites ±0   0 :zzz: ±0   1 files ±0   0 :x: ±0

Results for commit 45999ea5. ± Comparison against base commit 833af3b6.

:recycle: This comment has been updated with latest results.

codecov-commenter commented 2 years ago

Codecov Report

Merging #14 (87282cb) into main (833af3b) will increase coverage by 2.75%. The diff coverage is 81.35%.

@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
+ Coverage   76.15%   78.90%   +2.75%     
==========================================
  Files          24       25       +1     
  Lines         717      877     +160     
==========================================
+ Hits          546      692     +146     
- Misses        171      185      +14

Impacted Files	Coverage Δ
src/addition.rs	`92.10% <ø> (-1.00%)`	:arrow_down:
src/base_matrix.rs	`93.93% <ø> (ø)`
src/data_container.rs	`100.00% <ø> (ø)`
src/layouts/arbitrary_stride_column_major.rs	`0.00% <ø> (ø)`
src/layouts/arbitrary_stride_column_vector.rs	`0.00% <ø> (ø)`
src/layouts/arbitrary_stride_row_major.rs	`78.57% <ø> (ø)`
src/layouts/arbitrary_stride_row_vector.rs	`0.00% <ø> (ø)`
src/layouts/column_major.rs	`10.34% <ø> (ø)`
src/layouts/column_vector.rs	`51.61% <ø> (ø)`
src/layouts/row_major.rs	`89.65% <ø> (ø)`
... and 24 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

tbetcke commented 2 years ago

Thanks for this. It is probably not a bad idea to have a trait for floats that derives from Scalar so that we can easily extend functionality. I did not think too much about this so far.

I tried to look up meaningful extensions of atan2 to complex numbers. Naively, I would do the following:

atan2 is defined as the argument of X + iY. So if X and Y are complex I would still form Z=X + iY, and then call atan(Im(Z), Re(z)). The disadvantage is that this is not a complex analytic extension. So I am a bit hesitant to do this.

I checked how Matlab does it and they simply do not define atan2 for complex numbers. Given that keeping behaviour similar to Matlab is a good idea, I would suggest we do the same and simply use unimplemented!("atan2 is not defined for complex numbers); for the types c32 and c64.

strasdat commented 2 years ago

unimplemented!("atan2 is not defined for complex numbers);

Okay, I pushed that change. This PR is ready from my perspective.

In the long run, there might be some design iterations about the Scalar traits to consider. From my perspective, it feels a little awkward that cauchy::Scalar treats complex numbers as a first class citizen, and for real numbers one has to call .re() to retrieve it. But on the other hand this is just syntax, and hence not too big of a deal. One might also consider to have a ScalarBase trait and then e.g. a ScalarReal subtrait which contains atan2 and other functions which are only defined for real numbers (but not for complex ones).

UCL-ARC / householder

extending Scalar trait: atan2 #14

Test Results

Codecov Report