Skip to content

Conversation

@RyanWollaeger
Copy link
Collaborator

Description

Testing with a Debug build of a dependent code revealed a comparison of the same constants throws an assertion.

Changes

  • Modify memcmp usage in == overload comparing physical constants.
  • Update spiner submodule for fix of a recently added assert.

+ Modify memcmp usage in == overload comparing physical constants.
+ Update spiner submodule for fix of a recently added assert.
@RyanWollaeger RyanWollaeger requested a review from jonahm-LANL May 1, 2025 19:58
@RyanWollaeger
Copy link
Collaborator Author

@Yurlungur would it be reasonable to convert the bit-wise comparison in the overload to comparing the member data with tolerances (to permit, e.g. 1e-15 differences in units)? Or is that maybe not in the spirit of this comparison?

@Yurlungur
Copy link
Collaborator

@Yurlungur would it be reasonable to convert the bit-wise comparison in the overload to comparing the member data with tolerances (to permit, e.g. 1e-15 differences in units)? Or is that maybe not in the spirit of this comparison?

I think that would be fine. The current implementation is just laziness on my part I think.

@RyanWollaeger
Copy link
Collaborator Author

@Yurlungur would it be reasonable to convert the bit-wise comparison in the overload to comparing the member data with tolerances (to permit, e.g. 1e-15 differences in units)? Or is that maybe not in the spirit of this comparison?

I think that would be fine. The current implementation is just laziness on my part I think.

Will make that change quickly here then - thanks!

+ Add eps_equal private function to struct (formula from unit tests).
+ Use macro EPS value, similar to opacity classes.
@RyanWollaeger
Copy link
Collaborator Author

Just made the eps-comparison modification (the memcmp is much more succinct).

@Yurlungur Yurlungur merged commit 8649971 into lanl:main May 1, 2025
1 check passed
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.

2 participants