Skip to content

Conversation

@MatthewCochrane
Copy link
Contributor

Resolves #27 .

Checklist

Please ensure the following tasks are completed before submitting this pull request.

  • Read, understood, and followed the contributing guidelines, including the relevant style guides.
  • Read and understand the Code of Conduct.
  • Read and understood the licensing terms.
  • Searched for existing issues and pull requests before submitting this pull request.
  • Filed an issue (or an issue already existed) prior to submitting this pull request.
  • Rebased onto latest develop.
  • Submitted against develop branch.

Description

What is the purpose of this pull request?

This pull request:

  • Implements the expit function.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

Ended up using 1.5 * EPS for the tests as discussed in the gitter chat.


@stdlib-js/reviewers

@kgryte kgryte added Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality. labels Feb 16, 2019
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

@MatthewCochrane Wow! This PR looks great. Just some minor documentation changes and a possible benchmark bug. Once resolved, this should be ready for merge! Thanks for working on this!

kgryte and others added 2 commits February 16, 2019 20:08
Co-Authored-By: MatthewCochrane <mattisback@gmail.com>
Copy link
Contributor Author

@MatthewCochrane MatthewCochrane left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback. I've made the changes you requested and fixed that benchmarking bug. I haven't actually run the c code. I suppose i need to make install deps to get boost to be able to compile it? What's the purpose of the benchmarks in other languages by the way? I take it to compare with our javascript implementation? But there is no automated way to run the other language benchmarks yet?

@kgryte
Copy link
Member

kgryte commented Feb 16, 2019

@MatthewCochrane To run C benchmarks,

$ make benchmark-c BENCHMARKS_FILTER=.*/expit/.*

In order to run these C benchmarks, you do not need to install Boost. You only need Boost to run benchmarks against Boost, which the C benchmarks in this PR does not use.

Note: in order to compile C benchmarks, you will need gcc installed, which, if you are not on Windows, should not be required.

And you are correct. The benchmarks in the other languages are included in order to compare implementations to (1) reference implementations and (2) baseline performance implementations. In this case, including a C benchmark is useful for establishing baseline performance and assessing how much a JavaScript implementation deviates from the baseline.

Co-Authored-By: MatthewCochrane <matthew.cochrane.eng@gmail.com>
@MatthewCochrane
Copy link
Contributor Author

Ahh great. Good to know, thanks. Yep, makes sense.

@kgryte
Copy link
Member

kgryte commented Feb 16, 2019

@MatthewCochrane No problem! Taking a final look at the PR now!

@kgryte kgryte merged commit 3855ebf into stdlib-js:develop Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature Issue or pull request for adding a new feature. Math Issue or pull request specific to math functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFC: add the inverse logit function

2 participants