-
Notifications
You must be signed in to change notification settings - Fork 5
Add two versions of std math library, libcsm and phoenix #61
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
Conversation
Summary of ChangesHello @ayoopierre, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's mathematical capabilities by integrating a new, highly configurable standard math library. It provides the flexibility to choose between two distinct implementations, libmcs for critical applications and the existing libphoenix math, directly within the build system. This change is foundational, impacting how dependent projects link against the math library. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces the libmcs math library and integrates it into the build system. The changes are extensive, including the library's source code, build scripts, and documentation. While the overall structure is good, there are several issues in the Makefiles and CI configuration that need attention. Specifically, the libm/Makefile has issues with installing source files as headers, inconsistent indentation, and a typo. The .gitlab-ci.yml file has a potential race condition due to incorrect job dependencies. Additionally, there's a code style improvement suggested for a header file to avoid code bloat.
agkaminski
left a comment
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.
- pls add JIRA ID to commit msg
- add library in a separate commit from additional changes
3c59f95 to
2060f4a
Compare
Unit Test Results9 277 tests +63 8 687 ✅ +62 55m 5s ⏱️ +38s For more details on these failures, see this check. Results for commit ac6284d. ± Comparison against base commit 82ac411. ♻️ This comment has been updated with latest results. |
JIRA: RTOS-1132
JIRA: RTOS-1132
2060f4a to
21ce28b
Compare
21ce28b to
d5b9b25
Compare
JIRA: RTOS-1132
d5b9b25 to
ac6284d
Compare
|
Adding libmcs library, and tools for phoenix build system to select math library to build on (either libmcs or libphoenix implementation).
Description
Adding files from libmcs, move math files from libphoenix to core-libs, create new Makefile
Motivation and Context
Add math library for critical applications.
Types of changes
All projects, that depend on math library should add appropriate flag for linking against new library
How Has This Been Tested?
Checklist:
Special treatment