Skip to content

Update math constants and functions - #521

Merged
nkoukpaizan merged 10 commits into
developfrom
nicholson/math-updates
Aug 5, 2026
Merged

Update math constants and functions#521
nkoukpaizan merged 10 commits into
developfrom
nicholson/math-updates

Conversation

@nkoukpaizan

@nkoukpaizan nkoukpaizan commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Description

This updates the math constants and functions to used std::numbers.
Closes #95

Proposed changes

  • Removed #define _USE_MATH_DEFINES
  • Added #include <numbers> to Constants.hpp and removed unnecessary includes in other files.
  • Substituted M_PI for std::numbers::pi_v<RealT>
  • Ensured math functions used are from the std:: for the instances I could find.

Checklist

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • [N/A] There are unit tests for the new code.
  • [N/A] The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • [N/A] I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

@nkoukpaizan nkoukpaizan self-assigned this Aug 4, 2026
@nkoukpaizan nkoukpaizan added enhancement New feature or request cleanup labels Aug 4, 2026
@nkoukpaizan
nkoukpaizan marked this pull request as ready for review August 4, 2026 20:39
@nkoukpaizan
nkoukpaizan requested review from lukelowry and pelesh and removed request for pelesh August 4, 2026 20:39

@lukelowry lukelowry left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good cleanup, all mechanical is seems. Two non-issues I left a comment for

Comment thread GridKit/Model/PhasorDynamics/SynchronousMachine/GENROU/GenrouImpl.hpp Outdated
Comment thread GridKit/Model/PhasorDynamics/SynchronousMachine/GENSAL/GensalImpl.hpp Outdated
@nkoukpaizan
nkoukpaizan force-pushed the nicholson/math-updates branch from 8b427d3 to 1e9d88a Compare August 4, 2026 20:56

@pelesh pelesh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is good to merge.

@alexander-novo please check changes to Distributed Generator.

@nkoukpaizan
nkoukpaizan force-pushed the nicholson/math-updates branch from 6b0689a to 4a91e5f Compare August 4, 2026 23:34
@nkoukpaizan
nkoukpaizan merged commit 3f6cd2e into develop Aug 5, 2026
6 checks passed
andrewxu319 pushed a commit that referenced this pull request Aug 6, 2026
* Fix typo in expected errors message.

* Include number in Constants.hpp.

* Remove MATH_DEFINES, replace M_PI with std::numbers::pi_v; use std:: for math functions.

* Remove cmath include from PhasorDynamics models.

* Remove explicit cmath include from PhasorDynamics tests and apps.

* Make DG Jacobian implementation a little more readable.

* Use assign for DG rtemp instead of hard-coding repeated values.

* Fix machine equations alignment with pi.

* Additional spacing fixes for std::numbers::pi_v.

---------

Co-authored-by: nkoukpaizan <nkoukpaizan@users.noreply.github.com>
andrewxu319 pushed a commit that referenced this pull request Aug 6, 2026
* Fix typo in expected errors message.

* Include number in Constants.hpp.

* Remove MATH_DEFINES, replace M_PI with std::numbers::pi_v; use std:: for math functions.

* Remove cmath include from PhasorDynamics models.

* Remove explicit cmath include from PhasorDynamics tests and apps.

* Make DG Jacobian implementation a little more readable.

* Use assign for DG rtemp instead of hard-coding repeated values.

* Fix machine equations alignment with pi.

* Additional spacing fixes for std::numbers::pi_v.

---------

Co-authored-by: nkoukpaizan <nkoukpaizan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Include a central math header file for math constants

3 participants