-
-
Notifications
You must be signed in to change notification settings - Fork 794
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
bench: update benchmarks and examples in math/base/special/ldexp
#2781
Conversation
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
This error in examples comes even without applying the changes in this PR. I'll have a look on |
The error message comes as: example.c:20:10: fatal error: 'stdlib/math/base/special/ldexp.h' file not found
#include "stdlib/math/base/special/ldexp.h" even though the header files looks correct. |
@gunjjoshi Looks like |
lib/node_modules/@stdlib/math/base/special/ldexp/examples/c/example.c
Outdated
Show resolved
Hide resolved
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
@Planeshifter For other packages too, we do not include the package's path in the |
@gunjjoshi Yeah, the package itself shouldn't be listed as a dependency under its own configuration. I was mistakenly under the impression that the failure was caused in a different package's example. Not clear to me what's going on right now... Also seeing failures loading different modules in the CI checks here. |
@gunjjoshi |
@Planeshifter It doesn't seem we have used that package anywhere in |
We have used |
After looking into this a bit more, I noticed that running the C examples for Running the examples with cc @kgryte |
/stdlib merge |
Coverage Report
The above coverage report was generated for the changes in this PR. |
That's correct. We cannot handle cyclic dependencies in C implementations. In this case, I think it would be fine to refactor the example in order to avoid the use of |
stdlib
equivalent instead of built-in, fix indentation in math/base/special/ldexp
math/base/special/ldexp
@kgryte Yes, that does. I'll update the examples accordingly, thanks! |
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Signed-off-by: Gunj Joshi <[email protected]>
Updated the C examples. Also, updated benchmarks to follow latest conventions. |
The CI failure seems to occur due to the failure in |
@Planeshifter I forget. Did we not already address the Cephes build issue? |
math/base/special/ldexp
math/base/special/ldexp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks, @gunjjoshi.
@kgryte I don't recall us addressing the Cephes build errors. |
PR Commit Message
Please review the above commit message and make any necessary adjustments. |
As the CI failure is due to an upstream workflow failure and not due to the changes in this PR, I will go ahead and merge. |
PR-URL: #5378 Closes: stdlib-js/metr-issue-tracker#8 Ref: #2781 (comment) Reviewed-by: Athan Reines <[email protected]>
Description
This pull request:
pow
withstdlib_base_pow
.example.c
.Related Issues
None.
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers