Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add l1_norm() for Signed #43

Merged
merged 5 commits into from
Mar 28, 2019
Merged

Conversation

Schultzer
Copy link
Contributor

@Schultzer Schultzer commented Feb 25, 2019

Hi,
This adds the abs method for the FloatCore trait.

as far as I understand it there is no way to get the absolute value/norm of a complex number using no_std. with this method it should be possible to get absolute value with no_std.

let me know if you fine with the name abs() or want it to be norm() as pointed out in #38

The motivation is to be able to make https://github.com/schultzer/blas-rs compile with no_std.

/edit

This is actually "The 1-norm is simply the sum of the absolute values of the columns"

Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#38 pointed out that norm() already exists for the complex norm, which is also the absolute value.

src/lib.rs Outdated
@@ -434,6 +434,12 @@ impl<T: Clone + FloatCore> Complex<T> {
pub fn is_normal(self) -> bool {
self.re.is_normal() && self.im.is_normal()
}

/// Absolute value `re + im`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like the Manhattan distance from the origin, which is quite a different thing!

If you really want this, we could add it under a more specific name, though I would also choose T: Signed since it doesn't really need floating point at all.

Copy link
Contributor Author

@Schultzer Schultzer Feb 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would norm_1 be more ideal?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, do you have any precedent for that? I might suggest l1_norm, but l and 1 are visually confusing in many fonts.

Copy link
Contributor Author

@Schultzer Schultzer Feb 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, I'm no mathematician. I'm fine with l1_norm, I know in fortran they call it scabs1. I'm not really sure whats the right name is and don't really have any objection.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, now I see your edit also calling this 1-norm. Flipping that to norm_1 seems a little weird though. I'd like to know what other programming languages/libraries call this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a quick sample, it seems numpy, julia, and matlab all take an ord parameter to norm. I don't want a breaking change to our existing method though.

scalar_abs might be a modern take on fortran's scabs1. But if you're OK with it, l1_norm is more consistent with the existing norm. We can clarify that it's an "L1 norm" in the docs.

@Schultzer Schultzer changed the title Add abs() for FloatCore Add l1_norm() for Signed Feb 26, 2019
@Schultzer
Copy link
Contributor Author

Schultzer commented Feb 26, 2019

I've updated the pr and change the title accordingly to the name change to l1_norm() and for Signed

@Schultzer Schultzer mentioned this pull request Feb 26, 2019
3 tasks
@cuviper
Copy link
Member

cuviper commented Mar 27, 2019

bors r+

bors bot added a commit that referenced this pull request Mar 27, 2019
43: Add l1_norm() for Signed r=cuviper a=Schultzer

Hi,
This adds the abs method for the FloatCore trait.

as far as I understand it there is no way to get the absolute value/norm of a complex number using no_std. with this method it should be possible to get absolute value with no_std.

let me know if you fine with the name abs() or want it to be norm() as pointed out in #38 

The motivation is to be able to make https://github.com/schultzer/blas-rs compile with no_std.

/edit

This is actually ["The 1-norm is simply the sum of the absolute values of the columns"](https://en.wikipedia.org/wiki/Norm_(mathematics))

Co-authored-by: Benjamin Schultzer <benjamin@schultzer.com>
Co-authored-by: Josh Stone <cuviper@gmail.com>
@bors
Copy link
Contributor

bors bot commented Mar 28, 2019

Build succeeded

@bors bors bot merged commit 8bbdbfb into rust-num:master Mar 28, 2019
@Schultzer Schultzer deleted the add-abs-for-float-core branch May 1, 2019 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants