[jmitrevs]

Description

We discussed removing support for the deprecated Quartus (Intel HLS) backend. In particular, the table lookup implemented in that backend interferes to an extent with the softmax updates. This PR was created with the help of AI to do that (GPT 5.6 Sol).

Type of change

• Documentation update
• Other (Specify) -- remove old feature to ease maintenance

Tests

Quartus backend tests should be removed. Other things should pass.

Checklist

• I have read the guidelines for contributing.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have installed and run pre-commit on the files I edited or added.
• I have added tests that prove my fix is effective or that my feature works.

[calad0i]

Such refreshing line changes 😉

There is one failure in test_keras_api.py that I haven't seen before but also hard to be induced with the stated changes... Any idea on how it happened?

[JanFSchulte]

I have been wondering about that. The same test just passed for #1540 so I don't think we broke something in main. So the only difference is that the Quartus test isn't run before the oneAPI one. That seems to be pointing towards an issue with RNG and test orders that exposed some weird failure mode with the right set of inputs.

[JanFSchulte]

I think it's a feature of how we run the tests, based on an initial random seed that gets successively used by all tests in the CI job. So now that the oneAPI tests moved up one spot, it gets the random seed that previously was used by Quartus, and thus produces a result that is different from the baseline we have on file for it. So there is no issue with our code, but our test baselines are only valid for a fixed order of tests. We should probably change this so each test gets it's own fixed seed instead, and then re-record the baseline. But it's not blocking for this, or any other, PRs.

[jmitrevs]

Should we increase the error range for the comparison in test_rnn.py::test_rnn_accuracy[False-False-Bidirectional-Vivado-io_parallel-resource]?

[jmitrevs]

for test_keras_api I think we should make the input quantized by shifting and rounding explicitly. Then the match should be closer. I can try to add it. Though neither pytest fix is related to this PR, so I would be hesitant to add it here.

[JanFSchulte]

I think we shouldn't try to fudge these tests around the edges just to make them pass. If we touch them anyway, we should just implement the real fix of running each one with it's own random seed and re-generate the reference with that setup. That will prevent us from running into this again in the future if the tests get reordered again.

Where I agree is that this beyond the scope of this PR, so I'd be happy to merge it as it is.

[calad0i]

Second that we don't touch the tests for now if they are just random number induced.

[vloncar]

Given that we seed the CI with a fixed seed, doesn't this mean that the CI on main will be broken until something triggers the change in the split of tests in CI generator which will force the different ordering of tests, and thus different seed? Shouldn't be fixed here, but should be fixed in a PR that goes in before this one.

[JanFSchulte]

Yes, if we merge this, the tests on main will fail. So we would need to fix this before we merge. @marco66colombo is that something you can look at in terms of fixing the random seed for each synthesis test and regenerating the baseline?

[jmitrevs]

Quantizing the inputs (e.g. X = np.round(X * 2**10) * 2**-10 ) in general makes the tests more insensitive to random numbers. I wonder if that's the better fix. Ideally whether a test passes or fails shouldn't depend on the random number.

[marco66colombo]

@JanFSchulte I can look into that this weekend

[jmitrevs]

The oneAPI backend will go away as well, so I don't think we need to worry about utilzation error. Though truthfully if the Altera rename goes in first, there will probably be merge conflicts, and visa versa.

[JanFSchulte]

So this still has the problem that the tests rely on the order they are executed in within a job to get the correct random seed to reproduce the reference. So before it can be merged we should fix the random seed for each test within a suite for the synthesis test and generate a matching set of references. @marco66colombo Is that something you can find time for?