Skip to content

Conversation

breatn
Copy link

@breatn breatn commented Sep 28, 2025

This fix enables consistent behavior for linear transformations across all types of vector spaces in SageMath, resolving evaluation failures for vector spaces with combinatorial bases while maintaining full backward compatibility.

This fixes this bug: #40847

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.

⌛ Dependencies

@mantepse
Copy link
Contributor

Great! Could you also provide a doctest?

I am not very familiar with this part of sage, but I would have expected from the documentation that something like

sage: V = VectorSpace(QQ, range(3))
sage: im = [V.term(0), V.term(0), V.term(0)]
sage: H = Hom(V, V, category=sage.categories.vector_spaces.VectorSpaces(QQ))
sage: H(im)

should work, but it doesn't. Am I doing something wrong?

@mantepse
Copy link
Contributor

PS: unfortunately, I cannot approve workflow runs.

@breatn
Copy link
Author

breatn commented Sep 28, 2025

Well there are a few issues...

First, you'll want to use IntegerRange(3) instead of range(3):

sage: V = VectorSpace(QQ, IntegerRange(3))

For the images, if you map all basis elements to V.term(0), you'd get a transformation that collapses everything to a 1-dimensional space. For an identity-like transformation, you'd want:

sage: im = [V.term(0), V.term(1), V.term(2)]

Finally, without specifying the category, you'd get the wrong type of morphism. Here's what should work now:

  sage: V = VectorSpace(QQ, IntegerRange(3))
  sage: H = Hom(V, V, category=VectorSpaces(QQ))
  sage: im = [V.term(0), V.term(1), V.term(2)]  # identity mapping
  sage: lt = H(im)
  sage: type(lt)  # Should be VectorSpaceMorphism
  sage: lt(V.an_element())  # This used to crash, now works

I'll provide a doctest too :)

@mantepse
Copy link
Contributor

Well there are a few issues...

First, you'll want to use IntegerRange(3) instead of range(3):

Yes, I was sloppy.

For the images, if you map all basis elements to V.term(0), you'd get a transformation that collapses everything to a 1-dimensional space.

Yes, but that should be OK, it's a good linear transformation.

In any case, also the identity transformation doesn't work, if provided as a list of images.

@breatn
Copy link
Author

breatn commented Sep 28, 2025

I think it should work now, I ran some tests. Thanks for catching it out

breatn and others added 6 commits September 28, 2025 18:48
Co-authored-by: Martin Rubey <axiomize@yahoo.de>
Co-authored-by: Martin Rubey <axiomize@yahoo.de>
@breatn
Copy link
Author

breatn commented Sep 28, 2025

I think everything should be fine now. I fixed all the indentations and error messages to conform to the codebase.

@mantepse
Copy link
Contributor

sage -t src/sage/modules/vector_space_morphism.py gives a few errors with the current develop branch. After fixing these, you should also run sage -tox -- src/sage/modules/vector_space_morphism.py locally. Does that work for you?

@mantepse
Copy link
Contributor

@fchapoton, could you approve the workflow here?

@DolphinTechCodes
Copy link

Thank you very much for the quick fix!
One question: when generating the matrix in line 796, shouldn't the matrix be transposed then as the columns are the images of the basis vectors? Additionally, in case that side = "right", should the matrix be un- transposed? (i.e. transpose it when side = "left")

That aside, I am a bit confused why the use of the meathod C.coordinates works, as I thought only FreeModule but not FreeCombinatorialModule has it.

Furthermore, is there a shorter way to make the matrix sparse other than doing the matrix construction before calling linear_transformation ?

Thank you!

@breatn
Copy link
Author

breatn commented Sep 29, 2025

Um so maybe I did mess up for the matrix orientation

But c.coordinates does work because in Sage, both VectorSpace and CombinatorialFreeModule provide a .coordinates() method. So this usage is supported for both.

And on the making the matrix sparse You can pass sparse=True directly to the matrix constructor. Example:

matrix(D.base_ring(), D.dimension(), C.dimension(), ..., sparse=True)

@DolphinTechCodes
Copy link

Weirdly, i get án error when trying to call

V = VectorSpace(QQ, IntegerRange(3))
V.coordinates(V.an_element())

saying that 'CombinatorialFreeModule_with_category' object has no attribute 'coordinates'.

And concerning sparsity, I was referring to the case when one passes a list of images to linear_transformation. (so if one wants sparsity, one has to create the matrices manually)

@breatn
Copy link
Author

breatn commented Sep 29, 2025

Yes

The problem was that CombinatorialFreeModule (which is what VectorSpace(QQ, IntegerRange(3)) creates) didn't have a coordinates method, but the code assumed all vector spaces do.

I think it's solved

@mantepse
Copy link
Contributor

I think it's solved

I get many test failures now (using sage -t src/sage/modules/vector_space_morphism.py). Do the tests work for you?

@DolphinTechCodes
Copy link

Awesome,
I think that at some point in the future it may be convenient to implement a .coordinates() method in FreeCombinatorialModule that just raises an error if the basis is infinite-dimensional. It could even then return some sort of sparse vector, such that the matrix can be calculated then efficiently.

Meanwhile, do you think that it may be better for performance to use the to_vector method for FreeCombinatorialModule? E.g. in conjuntion with the sparse=True parameter that can be passed to the function?

@mantepse
Copy link
Contributor

mantepse commented Sep 29, 2025

I feel ignored.

@DolphinTechCodes, @breatn, could you please respond?

@DolphinTechCodes
Copy link

Sorry, I do not have any sage development environment set up, so i cannot tell you anything about tests (I just looked at the modified file)

@mantepse
Copy link
Contributor

Sorry, I do not have any sage development environment set up, so i cannot tell you anything about tests (I just looked at the modified file)

OK, although I don't quite understand. How are you testing your code then? In any case, it does not work at all, currently.

Could you please confirm that you are not a LLM? (with very few exceptions, we develop with our real name here)

@DolphinTechCodes
Copy link

Tbh, I just looked at the modified parts of the code and tested snippets in a jupyter notebook. I do not have a sage development environment set up so I do not actively make changes to the codebase, rather just 'consulting' bratn (who thankfully took on the effort to fix this issue).

Aside from that, I hereby confirm that I am not an LLM (however I dont know if that is sufficient proof for you). Is it really a requirement to have a github account linked to my name?

Copy link

Documentation preview for this PR (built with commit f124c66; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mantepse
Copy link
Contributor

Tbh, I just looked at the modified parts of the code and tested snippets in a jupyter notebook. I do not have a sage development environment set up so I do not actively make changes to the codebase, rather just 'consulting' bratn (who thankfully took on the effort to fix this issue).

OK. I don't see how a human could reasonably develop (or review other peoples code) without development environment. Thus, please refrain from praise (except specifically for the fact that someone is trying to fix something), it is misleading.

What is keeping you from compiling sage on your computer?

Aside from that, I hereby confirm that I am not an LLM (however I dont know if that is sufficient proof for you). Is it really a requirement to have a github account linked to my name?

I haven't met an LLM yet that would lie (without being instructed to) about itself. No, it's not a requirement, although I personally tend to trust accounts with real name more easily.

@breatn
Copy link
Author

breatn commented Sep 29, 2025

Im trying to download sage but there are so many things to download and the compilation took hours and hours.

@mantepse
Copy link
Contributor

Im trying to download sage but there are so many things to download and the compilation took hours and hours.

Then, most likely, something went wrong. What operating system?

@breatn
Copy link
Author

breatn commented Sep 29, 2025

macOS. But its an old intel computer

@mantepse
Copy link
Contributor

OK. Note that if you only modify .py files (as opposed to .pyx files) there is no need to recompile.

@breatn
Copy link
Author

breatn commented Sep 30, 2025

i've fixed the errors and it compiles on my machine

@breatn
Copy link
Author

breatn commented Sep 30, 2025

*and runs

@mantepse
Copy link
Contributor

I don't exactly know what you did, but it doesn't even get started here (and since this is python only, probably nowhere).

$ sage -t src/sage/modules/vector_space_morphism.py
...
ValueError: line 24 of the docstring for sage.modules.vector_space_morphism.VectorSpaceMorphism.__init__ has inconsistent leading whitespace: 'See the constructor,

If you look at the diff ("Files changed" tab on github), you will see that several blank lines were removed that shouldn't be removed. What editor are you using? (I recomment emacs + sage-shell-mode)

Please run the testsuite before claiming that it works!

(I am very grateful that you are working on the issue, I do realize that the road is very bumpy at the beginning. I hope I do not sound too harsh.)

@breatn
Copy link
Author

breatn commented Sep 30, 2025

Sorry. This might be obvious but why is there a segmentation error in the executable:

./sage

0 signals.cpython-312-darwin.so 0x0000000105756850 sigdie + 92
1 signals.cpython-312-darwin.so 0x00000001057566f4 cysigs_signal_handler + 368
2 libsystem_platform.dylib 0x000000019af9d6a4 _sigtramp + 56
3 error.cpython-312-darwin.so 0x0000000106760530 ZL58__pyx_pw_4sage_4libs_3ntl_5error_1setup_NTL_error_callbackP7_objectS0 + 24
4 libpython3.12.dylib 0x000000010515e0f8 PyObject_Vectorcall + 88
5 libpython3.12.dylib 0x000000010523cad0 PyEval_EvalFrameDefault + 17272
6 libpython3.12.dylib 0x0000000105238360 PyEval_EvalCode + 260
7 libpython3.12.dylib 0x000000010523604c builtin_exec + 1096
8 libpython3.12.dylib 0x00000001051a7384 cfunction_vectorcall_FASTCALL_KEYWORDS + 156
9 libpython3.12.dylib 0x000000010523d630 PyEval_EvalFrameDefault + 20184
10 libpython3.12.dylib 0x000000010515f6b4 object_vacall + 312
11 libpython3.12.dylib 0x000000010515f510 PyObject_CallMethodObjArgs + 104
12 libpython3.12.dylib 0x0000000105273008 PyImport_ImportModuleLevelObject + 1276
13 libpython3.12.dylib 0x0000000105234ff0 builtin___import
+ 276
14 libpython3.12.dylib 0x00000001051a7384 cfunction_vectorcall_FASTCALL_KEYWORDS + 156
15 libpython3.12.dylib 0x000000010523d630 _PyEval_EvalFrameDefault + 20184
16 libpython3.12.dylib 0x000000010515f6b4 object_vacall + 312
17 libpython3.12.dylib 0x000000010515f510 PyObject_CallMethodObjArgs + 104
18 libpython3.12.dylib 0x0000000105273008 PyImport_ImportModuleLevelObject + 1276
19 libpython3.12.dylib 0x000000010523b470 _PyEval_EvalFrameDefault + 11544
20 libpython3.12.dylib 0x0000000105238360 PyEval_EvalCode + 260
21 libpython3.12.dylib 0x000000010523604c builtin_exec + 1096
22 libpython3.12.dylib 0x00000001051a7384 cfunction_vectorcall_FASTCALL_KEYWORDS + 156
23 libpython3.12.dylib 0x000000010523d630 _PyEval_EvalFrameDefault + 20184
24 libpython3.12.dylib 0x000000010515f6b4 object_vacall + 312
25 libpython3.12.dylib 0x000000010515f510 PyObject_CallMethodObjArgs + 104
26 libpython3.12.dylib 0x0000000105273008 PyImport_ImportModuleLevelObject + 1276
27 libpython3.12.dylib 0x000000010523b470 _PyEval_EvalFrameDefault + 11544
28 libpython3.12.dylib 0x0000000105238360 PyEval_EvalCode + 260
29 libpython3.12.dylib 0x000000010523604c builtin_exec + 1096
30 libpython3.12.dylib 0x00000001051a7384 cfunction_vectorcall_FASTCALL_KEYWORDS + 156
31 libpython3.12.dylib 0x000000010523d630 _PyEval_EvalFrameDefault + 20184
32 libpython3.12.dylib 0x000000010515f6b4 object_vacall + 312
33 libpython3.12.dylib 0x000000010515f510 PyObject_CallMethodObjArgs + 104
34 libpython3.12.dylib 0x0000000105273008 PyImport_ImportModuleLevelObject + 1276
35 libpython3.12.dylib 0x000000010523b470 _PyEval_EvalFrameDefault + 11544
36 libpython3.12.dylib 0x0000000105238360 PyEval_EvalCode + 260
37 libpython3.12.dylib 0x00000001052924c4 run_mod + 168
38 libpython3.12.dylib 0x0000000105290c58 _PyRun_SimpleFileObject + 864
39 libpython3.12.dylib 0x00000001052906b8 _PyRun_AnyFileObject + 144
40 libpython3.12.dylib 0x00000001052b2d1c pymain_run_file + 320
41 libpython3.12.dylib 0x00000001052b2698 Py_RunMain + 1548
42 libpython3.12.dylib 0x00000001052b2898 pymain_main + 248
43 libpython3.12.dylib 0x00000001052b28e0 Py_BytesMain + 40
44 dyld 0x000000019abc2b98 start + 6076

Unhandled SIGSEGV: A segmentation fault occurred.
This probably occurred because a compiled module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.

zsh: segmentation fault ./sage

@mantepse
Copy link
Contributor

I'm sorry, I have no idea. I don't have a mac.

Did it work before you changed the source code?

@breatn
Copy link
Author

breatn commented Sep 30, 2025

This is the base source code. Even chatGPT and Claude can't help :(

@mantepse
Copy link
Contributor

sage-support, most likely. Or linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants