Skip to content

Add common angle/solid-angle units to units namespace#430

Merged
bartgol merged 10 commits intomasterfrom
bartgol/rad-units
Apr 14, 2026
Merged

Add common angle/solid-angle units to units namespace#430
bartgol merged 10 commits intomasterfrom
bartgol/rad-units

Conversation

@bartgol
Copy link
Copy Markdown
Contributor

@bartgol bartgol commented Apr 13, 2026

Motivation

rad is used a lot in E3SM, and users need to inject code like

Units rad (ekat::units::Units::nondimensional(),"rad")

which is not super-clear. It'd be clearer to just use ekat::units::rad directly.

Also, added a rename method, so one can do

auto m2 = (m*m).rename("m2");

which I think it's clearer (in intent) than

Units m2(m*m,"m2");

Finally, i removed dyn and rem, as they are non-SI units, but unlike g (which is used) they seem to be quite niche.

E3SM Stakeholder Feedback

Testing

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends EKAT’s units utility by adding commonly used angle/solid-angle symbols to the ekat::units namespace and introducing a convenience API for renaming derived units’ string representations.

Changes:

  • Added Units::rename(std::string_view) to retag a unit’s string representation while preserving its dimensions/scaling.
  • Added ekat::units::rad and ekat::units::sr as named dimensionless units for angle and solid angle.
  • Refactored several derived-unit declarations to use rename(...), and removed dyn and rem unit constants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bartgol bartgol force-pushed the bartgol/rad-units branch from 4d1d6d0 to 8a990b8 Compare April 13, 2026 17:35
@bartgol
Copy link
Copy Markdown
Contributor Author

bartgol commented Apr 14, 2026

This PR turned into a more general cleanup of the Units class and units namespace:

  • add rad, sr, and none constants
  • deprecate Units::nondimensional() (use constant instead.
  • deprecated rem and dyn (non-SI and VERY niche). Btw, dyn's numerical value was wrong, fixed it.
  • remove global array of basic units symbols
  • use string_view when possible
  • added is_scaled (parallel method to is_dimensionless).

@bartgol bartgol force-pushed the bartgol/rad-units branch 2 times, most recently from 2db069f to c0e9510 Compare April 14, 2026 16:59
@bartgol bartgol force-pushed the bartgol/rad-units branch from c0e9510 to 90b8089 Compare April 14, 2026 17:01
@mahf708
Copy link
Copy Markdown
Contributor

mahf708 commented Apr 14, 2026

shouldn't the derived units belong to the downstream app/customer?

@bartgol
Copy link
Copy Markdown
Contributor Author

bartgol commented Apr 14, 2026

shouldn't the derived units belong to the downstream app/customer?

That's one way to approach the issue. In general, yes, but I find it "reasonable" to provide some "common" ones (like N, Pa, J, W, etc).

@bartgol bartgol merged commit eb393cf into master Apr 14, 2026
4 checks passed
@bartgol bartgol deleted the bartgol/rad-units branch April 14, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants