Fix degree_complex #292
Merged
Fix degree_complex #292
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
More consistent behavior of degree_complex and halfdegree Make assertions into actual checks
blegat
reviewed
Mar 28, 2024
| halfdegree(t::AbstractTermLike) | ||
|
|
||
| Return the equivalent of `ceil(degree(t)/2)`` for real-valued terms or `degree_complex(t)` for terms with only complex | ||
| variables; however, respect any mixing between complex and real-valued variables. |
There was a problem hiding this comment.
Could you add an example section in the docstring to help the reader understand ?
blegat
reviewed
Mar 28, 2024
| """ | ||
| degree_complex(t::AbstractTermLike) | ||
|
|
||
| Return the _total complex degree_ of the monomial of the term `t`, i.e., the maximum of the total degree of the declared | ||
| variables in `t` and the total degree of the conjugate variables in `t`. | ||
| To be well-defined, the monomial must not contain real parts or imaginary parts of variables. | ||
| """ |
There was a problem hiding this comment.
The reader might not know what happens to this example: degree_complex(x^2 * conj(x) * y * conj(y)^2)). Is it 3 or 4 ? It might be useful as example to clarify.
|
Good point, added some examples. |
blegat
reviewed
Mar 30, 2024
src/complex.jl
Outdated
| """ | ||
| degree_complex(t::AbstractTermLike) | ||
|
|
||
| Return the _total complex degree_ of the monomial of the term `t`, i.e., the maximum of the total degree of the declared | ||
| variables in `t` and the total degree of the conjugate variables in `t`. | ||
| To be well-defined, the monomial must not contain real parts or imaginary parts of variables. | ||
| If `x` is a real-valued variable and `z` is complex-valued, | ||
| - `degree_complex(x^5) = 5` | ||
| - `degree_complex(z^3 * conj(z)^4) = 4` and `degree_complex(z^4 * conj(z)^3) = 4` |
There was a problem hiding this comment.
I meant also an example with two different complex-valued where one has a larger degree for the conjugate and not the other one
There was a problem hiding this comment.
Ok, now I use two real and two complex variables for the example, I hope this covers what you thought of.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the behavior of
degree_complexandhalfdegreeis inconsistent:degree_complexlumps real variables and declared complex variables together, whilehalfdegreecounts them separately. I would say that the second variant makes much more sense. This PR makes the behavior consistent: the complex degree is the sum of the degree of the real variables plus the maximum of the degrees of the declared complex vs. conjugated variables.Additionally, the methods contained some assertion-related checks; but they are not really assertions as it is perfectly valid for the user to define a monomial that contains both the variable and its real and imaginary parts individually. But for the purpose of complex degrees, this does not make sense, so we forbid it explicitly.