-
Notifications
You must be signed in to change notification settings - Fork 18
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
updated LinearAlgebraForCAP/gap/CompilerLogic.gi #1424
Conversation
bc8e96c
to
45f47ff
Compare
Codecov ReportAll modified lines are covered by tests ✅
📢 Thoughts on this report? Let us know!. |
5f7358b
to
42f43c1
Compare
This PR is difficult to review because it introduces so many rules at once, so it's difficult to see how the new compiled code exactly comes from the old code. Some rules might be suboptimal (e.g. Could you split the PR into smaller parts where the diff is more readable? |
You mean more several commits or several PRs? |
Depends :D If only view commits need further discussions, one PR is fine. If we would create too many threads, I would suggest splitting the PR. We can try with one PR. |
Done. Will update after #1432 is merged since I need the latter more urgently. |
42f43c1
to
e5ab128
Compare
Two rules turned out to be obsolete :) |
The first few commits only affect Maybe we can defer those optimizations only affecting niche applications for now? |
I am not sure if the first few logic templates only affect the operations you mentioned. It might be an artefact of the order of the commits. |
e5ab128
to
700d59a
Compare
You were right. The logic templates that did not depend on variables were only necessary to simplify Furthermore, I had to defer one rule, until its effect on the rest of the code becomes applicable. |
35f3348
to
2ec5219
Compare
Thinking more about this, I do not understand why the |
You are right :) |
2ec5219
to
756eae5
Compare
No need to merge this branch now. I just rebased to check it is still passing the CI. |
756eae5
to
5e5b8d1
Compare
5e5b8d1
to
6c7b8c5
Compare
Superseded by #1615. |
Preparing for the upcoming
PreComposeList
-PR.