Implement missing cmath functions

The following functions are currently missing:

Inspiration for how their implementation might look can be found in Modules/cmathmodule.c (and Modules/clinic/cmathmodule.c.h for argument parsing) of the CPython codebase. Lib/src/stdlib/math.rs equivalent functions should also be roughly similar.

Note to contributors: due to the way tests are organized, you'll need to add your function to the list of tested functions in the test_functions class attribute of CMathTests (on line 58 in Lib/tests/test_cmath.py). I.e, if you want to add log, add it
alphabetically at the list used in the comprehension:

# add it to the list
test_functions = [getattr(cmath, fname) for fname in ['cos', ..., 'log', 'sqrt']]

If anyone wants to pick a function or two up, leave a comment naming the functions so other people can see who's working on what. Hopefully that will minimize conflicts.

Hey i am very new but i would love to contribute could you tell me which files do i have to edit ?

@CTZxVULKAN

Hey @CTZxVULKAN the module is located at vm/src/stdlib/cmath.rs, you should probably look at other modules (like math.rs in the same directory) to get a feel for the type of constructs needed for defining a new built-in function.

After making your changes don't forget to test things out by running the Python tests with cargo run --release -- -m test (add -j8 to the end of that for a faster multicore execution).

Leave a comment on what you'll try and work on, though, so others can know.

nvm this is too complex for me rn.
i will comebacc later when i am more confident :(

You could drop me a message on gitter and I'll walk through most of it with you if you want. Everything is complex until you start doing mistakes with it and learning from them.

@DimitrisJim

@DimitrisJim

Hi I would like to pick up acos and acosh,
Though I'm very new to Rust so might require some guidance on the same.
In the math.rs module I see all the functions already implemented, for example:

// Trigonometric functions:
fn math_acos(x: IntoPyFloat, vm: &VirtualMachine) -> PyResult<f64> {
    let x = x.to_f64();
    if x.is_nan() || (-1.0_f64..=1.0_f64).contains(&x) {
        Ok(x.acos())
    } else {
        Err(vm.new_value_error("math domain error".to_owned()))
    }
}

On going through x.acos(), the implementation is:

#[must_use = "method returns a new number and does not mutate the original value"]
    #[stable(feature = "rust1", since = "1.0.0")]
    #[inline]
    pub fn acos(self) -> f64 {
        unsafe { cmath::acos(self) }
   }

Could you please guide me more, on what specifically needs to be implemented and where? This should give me a good heads up and help me get acquainted with the codebase.

Hm, you're currently looking at the implementation of cos for f64. cmath is concerned with complex numbers, Complex64 in our case from the num_complex crate.

cmath.isclose is another place you can look. It accepts things that can be transformed to Complex64 numbers (IntoPyComplex), calls to_complex to get the actual complex and then operates on it. You'd need to do something similar for acos.

@DimitrisJim

@DimitrisJim

Hello there, I would like to work on sqrt, sin and cos.

I have worked with C,Python and JS but have no experience using rust tbh tho.

Sure, go for it. If you get stuck at any point you can always drop by on gitter.

@DimitrisJim

@DimitrisJim
Hey I made a pull request would you please review it?

@DimitrisJim

@DimitrisJim Hey I implemented sin and cos,
@youknowone said something about sending a python file and using fetch and rebase instead of pull, can you please help me out,
I am not sure what to do

Just make sure you've updated your fork of Rustpython (to include the changes you made in the previous PR) and then base the new branch off that.

I don't see anything about sending a file (maybe the talk was about the test file being edited, which I'll guide you through once you've opened the PR).

@DimitrisJim

@DimitrisJim I updated my fork and created a pull request, Thankyou for being so patient and helpful!

I have no idea about sending file. it doesn't sound like something PR-related stuff.

fetch & rebase comment is this one: #3047 (comment)
For our workflow, merge commit must be only created by github. If any merge commit appears on PR, something goes wrong. That's commonly happend by git-merge or git-pull(as a shortcut of git-fetch && git-merge).

The good news is that we can rely on methods from the num-complex crate for pretty much all of these.

@egemengol

@egemengol sure, go for it. @Rishit-Khandelwal you can pick up acos then!

I actually just finished both and was going to submit the PR if that's okay

that's fine, I'll try and find another issue for Melih then.

Okay thanks 👍🏼

@mdegis

@mdegis see if #3182 is something you'd like to work on

@mdegis

@mdegis see if #3182 is something you'd like to work on

sure @DimitrisJim I would like that 👍

There is a problem for asin and acos.

When I take the asin and acos using the num-complex="0.4.0" like this, in a separate program:

let c = Complex64::new(0.2, 0.0);
let sin = c.asin();
let cos = c.acos();
println!("sin {:?} cos {:?}", sin.im, cos.im); // sin -0.0, cos -0.0

Just like we expect.

However, when we calculate the asin and acos of 0.2 in test_cmath.py:344, the imaginary part is 1.1102230246251565e-16 for both of them.

When I use CPython cmath, imaginary part is 0. When I use RustPython cmath with cargo run, the asin or acos functions return the value above.

I wonder if this is related to floating point arithmetic errors? Does anyone have an idea on how to handle this?

99% a rounding error that propagates. Don't worry too much about it, there's other edge cases we currently do not handle in a correct way yet.

Add a TODO: in the function body describing the issue in a few words for now and we'll get to it at some later point.

Whatever it is, it's pretty weird since using the log forms for calculating it (as num-complex uses) yields a similar result:

def ln(c):
    r, th = cmath.polar(c)
    return complex(math.log(r), th)

def asin(c):
    one, i = complex(1), complex(0, 1)
    return -i * ln(cmath.sqrt(one - c*c) + i*c)

Both CPython and RustPython return: (0.20135792079033082+1.1102230246251565e-16j).

@egemengol

@egemengol @Rishit-Khandelwal there's an unfortunate conflict between your PRs (#3198 and #3200). Since we can't push both one is going to have to be closed. I'll be able to find another good issue for the other person to contribute to make up for it.

As it stands, @Rishit-Khandelwal has the fact that they requested to contribute first which we should respect (though I do believe @egemengol has touched a bit more on some edgecases!).

Sorry for this but it happens when a big issue like this might be opened.

@DimitrisJim

Ah, I missed #3198, my bad :)

Although I think communicating a bit more would prevent this duplication of work, no worries.

Thanks @DimitrisJim for sincere consideration. I'm definitely willing to contribute more, see you guys in different threads :)

Although I think communicating a bit more would prevent this duplication of work

Yup, I agree there.

I'll try and find a new issue for you though. If you have any particular interests, of course, you can grep for TODO: RUSTPYTHON in test files that interest you and try and tackle something from there (though I can't guarantee it will be easy 😄)

@egemengol

@egemengol see if #3217 might interest you.

Thank you all!

Navigation Menu

Repository navigation

Implement missing `cmath` functions #3039

10 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Implement missing cmath functions #3039

Description

Activity

CTZxVULKAN commented on Sep 11, 2021

DimitrisJim commented on Sep 11, 2021

CTZxVULKAN commented on Sep 11, 2021

DimitrisJim commented on Sep 11, 2021

hk1997 commented on Sep 11, 2021

DimitrisJim commented on Sep 11, 2021

Codemonk-adi commented on Sep 13, 2021

DimitrisJim commented on Sep 13, 2021

Codemonk-adi commented on Sep 13, 2021

Codemonk-adi commented on Sep 14, 2021

DimitrisJim commented on Sep 14, 2021

Codemonk-adi commented on Sep 14, 2021

youknowone commented on Sep 14, 2021

fanninpm commented on Sep 14, 2021

10 remaining items

DimitrisJim commented on Oct 1, 2021

emhagman commented on Oct 1, 2021

DimitrisJim commented on Oct 1, 2021

emhagman commented on Oct 1, 2021

DimitrisJim commented on Oct 1, 2021

mdegis commented on Oct 1, 2021

egemengol commented on Oct 1, 2021

DimitrisJim commented on Oct 1, 2021

DimitrisJim commented on Oct 1, 2021

DimitrisJim commented on Oct 2, 2021

egemengol commented on Oct 2, 2021

DimitrisJim commented on Oct 2, 2021

DimitrisJim commented on Oct 4, 2021

DimitrisJim commented on Oct 6, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Implement missing `cmath` functions #3039