Skip to content

Inconsistent function signature for finite rings sqrt / square_root #40796

@vincentmacri

Description

@vincentmacri

The function signature and default values for parameters for square roots of finite rings/fields is inconsistent. @user202729 pointed out this is a violation of the Liskov substitution principle. This was discussed in #40401 and determined to be out of scope for that PR.

A few examples of the inconsistencies:

What values make sense for the algorithm parameter will vary between implementations. For that parameter specifically I think the various implementations should fallback to their default square root algorithm if the user passes an algorithm option that is not supported there. For example, algorithm=tonelli is supported for the finite field category implementation but doesn't make sense for the Givaro implementation because the Givaro square root algorithm is so efficient that there is no point in using anything else. So I suggest that Givaro should have an optional algorithm parameter to maintain consistency (Liskov substitution principle) but ignore it.

Since the algorithm parameter changes how something is computed but not what is returned, we can have different supported algorithms for the various implementations without worrying about violating the Liskov substitution principle. If we fallback to a default algorithm if the user asks to use an unknown algorithm, we could (not saying we should without a good reason) even remove algorithms without going through a deprecation period, as the code will still behave the same.

Unless there is a good reason to do otherwise, I think the default behaviour of extend and all should be consistent for all finite rings. This may require a some kind of deprecation period.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions