AbstractAlgebra.jl icon indicating copy to clipboard operation
AbstractAlgebra.jl copied to clipboard

Compute Solve traits from types as args

Open lgoettgens opened this issue 7 months ago • 4 comments

My motivation for this is to be able to set a concrete type in the @attr Any call in https://github.com/oscar-system/Oscar.jl/pull/4915/files#diff-88ef12a4e6f091b2cc3dd521d7c8be444dab854cb7aef78f6837778a274315ffR489 (something along the lines of @attr solve_context_type(base_ring_type(M))).

I also copied over the two fallbacks (for ::Any and for ::DataType) over from all of the similar functions like base_ring_type and then removed some obsolete methods.

@joschmitt does this change make sense to you?

lgoettgens avatar May 28 '25 09:05 lgoettgens

Codecov Report

Attention: Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.36%. Comparing base (cd318ff) to head (4d0126d).

Files with missing lines Patch % Lines
src/Solve.jl 83.33% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2101      +/-   ##
==========================================
- Coverage   88.36%   88.36%   -0.01%     
==========================================
  Files         126      126              
  Lines       31656    31660       +4     
==========================================
+ Hits        27974    27975       +1     
- Misses       3682     3685       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 28 '25 10:05 codecov[bot]

I think the idea was that the "best" matrix normal form does not only depend on the type. For example, a zzModRing might be a field in which case the flint RREF (or LU?) should be better than the Howell form. That being said, we never seemed to have followed through with this idea.

@thofma what do you think?

joschmitt avatar May 28 '25 10:05 joschmitt

Yes, the idea was to combine it with something like is_known(F, :is_field) and to use RREF in this case (make it a runtime thing). We might have to untangle this somehow, but I don't know how yet. Having one type for all solve contexts (with some flag) won't work, since it needs to be extensible.

thofma avatar May 30 '25 06:05 thofma

Yes, the idea was to combine it with something like is_known(F, :is_field) and to use RREF in this case (make it a runtime thing). We might have to untangle this somehow, but I don't know how yet. Having one type for all solve contexts (with some flag) won't work, since it needs to be extensible.

Hmm, I don't really see how to do that without making inference of all of solving impossible. But I am open to any ideas you might have on the matter

lgoettgens avatar May 30 '25 23:05 lgoettgens