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 utils/include/edm4eic/unit_system.h #35

Merged
merged 6 commits into from
Jul 31, 2023
Merged

Conversation

veprbl
Copy link
Member

@veprbl veprbl commented May 3, 2023

Defines constants to support EDM4hep's unit system to be adopted as the default for EICrecon.

@c-dilks
Copy link
Member

c-dilks commented May 3, 2023

  • length unit is mm := 1
  • choose either full SI prefix, abbreviation, or both; e.g., millimeter vs. mm, or microsecond vs us
  • some comments are incorrect, e.g. // mega electron volt on eV

@veprbl
Copy link
Member Author

veprbl commented May 3, 2023

  • length unit is mm := 1

Fixed

  • choose either full SI prefix, abbreviation, or both; e.g., millimeter vs. mm, or microsecond vs us

I'm trying to chose abbreviation here. C++ doesn't support full unicode to use $\mu$. This is made minimal on purpose to avoid introducing alternative ways to do things.

  • some comments are incorrect, e.g. // mega electron volt on eV

Fixed

@c-dilks
Copy link
Member

c-dilks commented May 4, 2023

Perhaps this would be better in EDM4hep, for everyone's benefit.

@veprbl
Copy link
Member Author

veprbl commented May 16, 2023

Filed downstream PR eic/EICrecon#671 to start using this.

@wdconinc
Copy link
Contributor

Only noticed this PR now. This looks good.

@wdconinc wdconinc merged commit bf5b8cc into main Jul 31, 2023
@wdconinc wdconinc deleted the pr/add_unit_system branch July 31, 2023 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants