Skip to content
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

Add remaining exact SI constants #336

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Conversation

chiphogg
Copy link
Contributor

@chiphogg chiphogg commented Dec 2, 2024

All values were taken from this section of the wikipedia page on the
2019 revision of the SI.

Admittedly, two of them --- Delta_nu_Cs and K_cd --- have horribly
awkward names. The best I could do for the constant names was
CESIUM_HYPERFINE_TRANSITION_FREQUENCY and
LUMINOUS_EFFICACY_540_TERAHERTZ, respectively. That's fine; we expect
most users to do something like this in their programs:

constexpr auto K_cd = au::LUMINOUS_EFFICACY_540_TERAHERTZ;

Helps #90.

@chiphogg chiphogg added the release notes: 📏 lib (new units/constants) PR adding new units or constants to the library label Dec 2, 2024
@chiphogg chiphogg marked this pull request as draft December 2, 2024 16:09
@chiphogg
Copy link
Contributor Author

chiphogg commented Dec 2, 2024

CMake tests are failing; not sure why yet.

chiphogg added a commit that referenced this pull request Dec 2, 2024
Turns out, using `IToA` for magnitudes was a little bit sloppy.
Magnitude values are always unsigned, which means there are meaningful
values that won't fit inside of an `int64_t`.  Fortunately, adding all
those constants (#336) was a great stress test to expose this.

I think the most natural fix would be to add a `UIToA`, which can't
handle signed values, but which _can_ handle the "upper half" of
`uint64_t` values.  This way, we can have `IToA` delegate to `UIToA` for
the "integer magnitude" parts of the logic.

Why not just get rid of `IToA` altogether?  Well, we do need it for
printing exponents, which can be negative.

Helps #90: this gets the test to pass on #336.
@chiphogg chiphogg mentioned this pull request Dec 2, 2024
@chiphogg chiphogg changed the base branch from main to chiphogg/uitoa#90 December 2, 2024 17:01
@chiphogg
Copy link
Contributor Author

chiphogg commented Dec 2, 2024

PR Branch Title
#337 chiphogg/uitoa#90 Add UIToA<uint64_t>
➡️ chiphogg/exact-si-constants#90 Add remaining exact SI constants

Autogenerated by bonsai, do not edit

chiphogg added a commit that referenced this pull request Dec 2, 2024
Turns out, using `IToA` for magnitudes was a little bit sloppy.
Magnitude values are always unsigned, which means there are meaningful
values that won't fit inside of an `int64_t`.  Fortunately, adding all
those constants (#336) was a great stress test to expose this.

I think the most natural fix would be to add a `UIToA`, which can't
handle signed values, but which _can_ handle the "upper half" of
`uint64_t` values.  This way, we can have `IToA` delegate to `UIToA` for
the "integer magnitude" parts of the logic.

Why not just get rid of `IToA` altogether?  Well, we do need it for
printing exponents, which can be negative.

Helps #90: this gets the test to pass on #336.
Base automatically changed from chiphogg/uitoa#90 to main December 2, 2024 17:44
@chiphogg chiphogg marked this pull request as ready for review December 2, 2024 18:03
@chiphogg
Copy link
Contributor Author

chiphogg commented Dec 2, 2024

CMake tests are failing; not sure why yet.

Answer: we needed #337. All good now.

All values were taken from [this section] of the wikipedia page on the
2019 revision of the SI.

Admittedly, two of them --- `Delta_nu_Cs` and `K_cd` have horribly
awkward names.  The best I could do for the constant names was
`CESIUM_HYPERFINE_TRANSITION_FREQUENCY` and
`LUMINOUS_EFFICACY_540_TERAHERTZ`, respectively.  That's fine; we expect
most users to do something like this in their programs:

```cpp
constexpr auto K_cd = au::LUMINOUS_EFFICACY_540_TERAHERTZ;
```

[this section]: https://en.wikipedia.org/wiki/2019_revision_of_the_SI#Defining_constants
@chiphogg chiphogg force-pushed the chiphogg/exact-si-constants#90 branch from a53d0c6 to 48795e8 Compare December 2, 2024 18:46
Copy link
Contributor

@geoffviola geoffviola left a comment

Choose a reason for hiding this comment

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

Looks good. I did some spot checks against the wikipedia page and it seems to match.

@chiphogg chiphogg merged commit b955046 into main Dec 2, 2024
13 checks passed
@chiphogg chiphogg deleted the chiphogg/exact-si-constants#90 branch December 2, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: 📏 lib (new units/constants) PR adding new units or constants to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants