updated LinearAlgebraForCAP/gap/CompilerLogic.gi

[mohamed-barakat]

Preparing for the upcoming PreComposeList-PR.

[codecov[bot]]

Codecov Report

All modified lines are covered by tests :white_check_mark:

Files	Coverage Δ
LinearAlgebraForCAP/PackageInfo.g	100.00% <100.00%> (ø)
LinearAlgebraForCAP/gap/CompilerLogic.gi	100.00% <100.00%> (ø)
...recompiled_categories/MatrixCategoryPrecompiled.gi	66.52% <ø> (+0.47%)	:arrow_up:

:loudspeaker: Thoughts on this report? Let us know!.

[zickgraf]

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. KroneckerMat(id, id) -> id is potentially a loss of information, although I guess here it does not cause problems) and others need filters (e.g. 0 * something = 0 does not hold if something is a homalg matrix).

Could you split the PR into smaller parts where the diff is more readable?

[mohamed-barakat]

You mean more several commits or several PRs?

[zickgraf]

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.

[mohamed-barakat]

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 https://github.com/homalg-project/CAP_project/pull/1432 is merged since I need the latter more urgently.

[mohamed-barakat]

Done. Will update after #1432 is merged since I need the latter more urgently.

https://github.com/homalg-project/CAP_project/pull/1433 is hopefully the last one I need before rebasing this PR.

[mohamed-barakat]

Two rules turned out to be obsolete :)

[zickgraf]

The first few commits only affect MorphismFromFiberProductToSink etc., which I consider niche applications. I'm a bit reluctant specializing too much for such special applications and thus slow down the whole compilation process, especially if the optimizations could/should be implemented much more generically. For example, the first two rules could be handled generically by "If an expression does not depend on variables and is a nested list of literals (e.g. integers), evaluate it and use the result." Here, "using the result" means encoding the result as a syntax tree again, which is something you already do in the context of the ur-algorithms, I think.

Maybe we can defer those optimizations only affecting niche applications for now?

[mohamed-barakat]

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.

[mohamed-barakat]

The first few commits only affect MorphismFromFiberProductToSink etc., which I consider niche applications. I'm a bit reluctant specializing too much for such special applications and thus slow down the whole compilation process, especially if the optimizations could/should be implemented much more generically. For example, the first two rules could be handled generically by "If an expression does not depend on variables and is a nested list of literals (e.g. integers), evaluate it and use the result."

You were right. The logic templates that did not depend on variables were only necessary to simplify MorphismFromFiberProductToSink etc.

Furthermore, I had to defer one rule, until its effect on the rest of the code becomes applicable.

[zickgraf]

Thinking more about this, I do not understand why the PreComposeList-PR should change any compiled code (except the one of PreComposeList of course). I don't think the additional source and range are used except for the empty case, but this case does not occur in the existing code. So maybe the changes were an artifact of the implementation of PreComposeList via if-else instead of using Iterated with three arguments?

[mohamed-barakat]

So maybe the changes were an artifact of the implementation of PreComposeList via if-else instead of using Iterated with three arguments?

You are right :)

[mohamed-barakat]

No need to merge this branch now. I just rebased to check it is still passing the CI.

[mohamed-barakat]

After merging https://github.com/homalg-project/CAP_project/pull/1492 I had to update the PR. Commit c8f27fbafad476d463b0e3c47deb2b3e58c380d1 made one commit in this PR obsolete.

[mohamed-barakat]

Superseded by https://github.com/homalg-project/CAP_project/pull/1615.