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

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

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

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
Hey I made a pull request would you please review it?

@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 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.

I think some (like log10?) aren't there but most do just forward the call. That's why they're a pretty good first issue.

Hello, I would like to work on tan and tanh :)
@youknowone @DimitrisJim

go for it @tony-jinwoo-ahn

@DimitrisJim I'd like to work on sinh and cosh if possible!

@emhagman great, work on them!

Hey I would also love to contribute, what should work on?

acos and/or asin appear to be without anyone currently working on them @Rishit-Khandelwal

I would like to start working on asin if it is still not implemented

If it is ok, I can try to contribute cosh function 👨‍🔬

seems like Eric has eyes on cosh @mdegis. @emhagman you fine with working with sinh while Melih works on cosh?

@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 see if #3182 is something you'd like to work on

@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 @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.

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 see if #3217 might interest you.

Thank you all!