-
Notifications
You must be signed in to change notification settings - Fork 98
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
Conversion to/from complex storage types is wrong #452
Comments
mkalte666
changed the title
Conversion to/from complex storage types is weong
Conversion to/from complex storage types is wrong
Jan 15, 2024
mkalte666
added a commit
to mkalte666/uom
that referenced
this issue
Jan 15, 2024
Initially, `ConversionFactor` and thus `Conversion::T` requred `PartialEq`. This makes sense for the conversion factor itself (i.e. scaling across units), however it breaks once you introduce complex numbers. Those can *still* be scaled just like normal numbers - you essentially just increase or decrese a vector length, but the conversion function cannot compare them - "Z_1 < Z_2" is not trivially decidable. It is, however, also not needed - unit scales are just that - scalars that scale. And those can be easily compared. This commit seperates `Conversion::T` into `Conversion::VT` and `Conversion::T` and moves the `PartialEq` requirements from `ConversionFactor` into `Conversion::TT` directly. This requires a lot of trait bounds added down the line, so im not 100% that this does not break anything down the line. There might be a nicer way to go about this, but i haven't found any. closes iliekturtles#452
mkalte666
added a commit
to mkalte666/uom
that referenced
this issue
Oct 14, 2024
Initially, `ConversionFactor` and thus `Conversion::T` requred `PartialEq`. This makes sense for the conversion factor itself (i.e. scaling across units), however it breaks once you introduce complex numbers. Those can *still* be scaled just like normal numbers - you essentially just increase or decrese a vector length, but the conversion function cannot compare them - "Z_1 < Z_2" is not trivially decidable. It is, however, also not needed - unit scales are just that - scalars that scale. And those can be easily compared. This commit seperates `Conversion::T` into `Conversion::VT` and `Conversion::T` and moves the `PartialEq` requirements from `ConversionFactor` into `Conversion::TT` directly. This requires a lot of trait bounds added down the line, so im not 100% that this does not break anything down the line. There might be a nicer way to go about this, but i haven't found any. closes iliekturtles#452
mkalte666
added a commit
to mkalte666/uom
that referenced
this issue
Oct 14, 2024
Initially, `ConversionFactor` and thus `Conversion::T` requred `PartialEq`. This makes sense for the conversion factor itself (i.e. scaling across units), however it breaks once you introduce complex numbers. Those can *still* be scaled just like normal numbers - you essentially just increase or decrese a vector length, but the conversion function cannot compare them - "Z_1 < Z_2" is not trivially decidable. It is, however, also not needed - unit scales are just that - scalars that scale. And those can be easily compared. This commit seperates `Conversion::T` into `Conversion::VT` and `Conversion::T` and moves the `PartialEq` requirements from `ConversionFactor` into `Conversion::TT` directly. This requires a lot of trait bounds added down the line, so im not 100% that this does not break anything down the line. There might be a nicer way to go about this, but i haven't found any. closes iliekturtles#452
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Heya, i have run into the following issue:
Lets assume a complex impedance like
Z = (123 + 321j) * ohm
this fails:
343.7586362551492
being the norm of the number.I have looked at the implementing PR: while i would expect the pure unit conversion factor to be a floating point type, the actual conversion operation must be in the complex numbers, otherwise the operation is one-way (there are infinite solutions to
|z|^2 = re^2 + im^2
for a single given z; think vectors describing a circle of radius z).Im gonna have a go at either understanding where im wrong in my assumptions, or fixing this, as i really want complex impedance :D
The text was updated successfully, but these errors were encountered: