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

Added missing trigonometric functions(sech , csch , acsch) #792

Merged
merged 8 commits into from Feb 13, 2016

Conversation

Projects
None yet
4 participants
@CodeMaxx
Copy link
Contributor

CodeMaxx commented Feb 6, 2016

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:trigo branch from 116325c to cf0f856 Feb 6, 2016

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:trigo branch from 666e8f1 to 3868fad Feb 6, 2016

static RCP<const Basic> diff(const ACsch &self,
const RCP<const Symbol> &x) {
return mul(div(minus_one, mul(sqrt(add(one, pow(self.get_arg(), i2))),
abs(self.get_arg()))), self.get_arg()->diff(x));

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

This is not true for complex arguments. Use -1/(x^2*sqrt(1/x^2 + 1))

This comment has been minimized.

@CodeMaxx

CodeMaxx Feb 6, 2016

Contributor

Chaged.

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 6, 2016

I think you forgot to implement the class ACsch in functions.h

//! \return `true` if canonical
bool is_canonical(const RCP<const Basic> &arg) const;
//! \return Canonicalized acsch
virtual RCP<const Basic> create(const RCP<const Basic> &arg) const;

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

Is this implemented?

This comment has been minimized.

@CodeMaxx

CodeMaxx Feb 8, 2016

Contributor

Done. Had missed this somehow.

@@ -734,6 +803,8 @@ class ACosh: public HyperbolicFunction {
//! Canonicalize ACosh:
RCP<const Basic> acosh(const RCP<const Basic> &arg);



This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

Remove these unnecessary lines

virtual RCP<const Basic> sech(const Basic &x) const override {
SYMENGINE_ASSERT(is_a<T>(x))
return number(std::sech(static_cast<const T &>(x).i));
}

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

Can you add acsch as well?

mpfr_class t(static_cast<const RealMPFR &>(x).i.get_prec());
mpfr_sech(t.get_mpfr_t(), static_cast<const RealMPFR &>(x).i.get_mpfr_t(), MPFR_RNDN);
return real_mpfr(std::move(t));
}

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

Same here

return false;
}

int Csch::compare(const Basic &o) constraints

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

constraints to const

}
}
if (could_extract_minus(arg)) {
return csch(neg(arg));

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

This should be neg(csch(neg(arg)))

if (not _arg->is_exact()) {
return _arg->get_eval().csch(*_arg);
} else if (_arg->is_negative()) {
return csch(zero->sub(*_arg));

This comment has been minimized.

@isuruf

isuruf Feb 6, 2016

Member

Same here

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 7, 2016

@isuruf I'll fix this in by thursday. Have an exam.

SYMENGINE_ASSERT(is_a<ComplexMPC>(x))
mpc_class t(static_cast<const ComplexMPC &>(x).i.get_prec());
mpc_ui_div(t.get_mpc_t(), 1, static_cast<const ComplexMPC &>(x).i.get_mpc_t(), MPFR_RNDN);
mpc_atanh(t.get_mpc_t(), t.get_mpc_t(), MPFR_RNDN);

This comment has been minimized.

@isuruf

isuruf Feb 8, 2016

Member

Is this correct?

This comment has been minimized.

@CodeMaxx

CodeMaxx Feb 8, 2016

Contributor

No its not. Should be asinh.

virtual RCP<const Basic> acsch(const Basic &x) const override {
SYMENGINE_ASSERT(is_a<RealMPFR>(x))
mpfr_srcptr x_ = static_cast<const RealMPFR &>(x).i.get_mpfr_t();
if (mpfr_cmp_si(x_, 1) >= 0 or mpfr_cmp_si(x_, -1) <= 0) {

This comment has been minimized.

@isuruf

isuruf Feb 8, 2016

Member

this check is unnecessary

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:trigo branch from c61b01b to 9b69448 Feb 8, 2016

@CodeMaxx CodeMaxx force-pushed the CodeMaxx:trigo branch from 9b69448 to bca8715 Feb 8, 2016

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 8, 2016

Looks good to me. Great job.
Thanks for being very thorough and implementing all of the functionality.

@CodeMaxx CodeMaxx changed the title [WIP] added missing trigonometric functions(sech , csch , acsch) Added missing trigonometric functions(sech , csch , acsch) Feb 8, 2016

@certik

This comment has been minimized.

Copy link
Contributor

certik commented Feb 8, 2016

Should we add at least some tests for the new classes?

@srajangarg

This comment has been minimized.

Copy link
Contributor

srajangarg commented Feb 8, 2016

Yes, you can add the same test cases as of the other (similar) functions.

@CodeMaxx

This comment has been minimized.

Copy link
Contributor

CodeMaxx commented Feb 8, 2016

I'll do it by tomorrow

CodeMaxx added some commits Feb 10, 2016

@certik

This comment has been minimized.

Copy link
Contributor

certik commented Feb 13, 2016

This looks good to me. @isuruf any objections?

@isuruf

This comment has been minimized.

Copy link
Member

isuruf commented Feb 13, 2016

Looks good to me

certik added a commit that referenced this pull request Feb 13, 2016

Merge pull request #792 from CodeMaxx/trigo
Added missing trigonometric functions(sech , csch , acsch)

@certik certik merged commit 2502a42 into symengine:master Feb 13, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@CodeMaxx CodeMaxx referenced this pull request Feb 13, 2016

Closed

Add missing trigonometric functions #777

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment