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.

Sign up for GitHub

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

Jump to bottom

Fix uniform field unit conversion #931

Merged
merged 1 commit into from
Sep 8, 2023
Merged

Fix uniform field unit conversion #931

merged 1 commit into from
Sep 8, 2023

Conversation

amandalund
Copy link
Contributor

Well that's why our celer-g4 performance results with a uniform field looked suspiciously good...

@amandalund amandalund added the bug Something isn't working label Sep 8, 2023
@amandalund amandalund requested a review from sethrj September 8, 2023 17:10
sethrj
sethrj approved these changes Sep 8, 2023
View reviewed changes
Copy link
Member

@sethrj sethrj left a comment

Choose a reason for hiding this comment

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

My bad for missing this in the last review... is it possible to add a unit test for the along-step factory? At some point we'll need to add something more than a "does it run" test for the front end apps too.

@sethrj sethrj added the app Application front ends label Sep 8, 2023
@amandalund
Copy link
Contributor Author

I don't think I changed the field units when I refactored for the json input (except for messing up the magnitude_tesla), but I definitely should have caught this. Yeah, an along-step factory test is a good idea (and agreed about our app tests).

@sethrj
Copy link
Member

sethrj commented Sep 8, 2023

Is this bug present in v0.3?

@amandalund
Copy link
Contributor Author

Yep, looks like it is.

@sethrj
Copy link
Member

sethrj commented Sep 8, 2023

Damn, you're right. Thanks for the catch.

@sethrj sethrj merged commit d98ea16 into celeritas-project:develop Sep 8, 2023
@amandalund amandalund deleted the fix-field-units branch September 8, 2023 18:46
sethrj pushed a commit that referenced this pull request Sep 8, 2023
@amandalund @sethrj
Fix uniform field unit conversion (#931)
e013143
@sethrj
Copy link
Member

sethrj commented Sep 8, 2023

Yeah, the unit error was in the original #702 . It just got moved in #890 .

@sethrj sethrj added the backport Pull request duplicated across version branches label Sep 14, 2023
@sethrj sethrj added performance Changes for performance optimization and removed performance Changes for performance optimization labels Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sethrj sethrj sethrj approved these changes

Assignees
No one assigned
Labels
app Application front ends backport Pull request duplicated across version branches bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amandalund @sethrj