Failures in test_sr.py

[jmitrevs]

Prerequisites

Please make sure to check off these prerequisites before submitting a bug report.

• Test that the bug appears on the current version of the master branch. Make sure to include the commit hash of the commit you checked out.
• Check that the issue hasn't already been reported, by checking the currently open issues.
• If there are steps to reproduce the problem, make sure to write them down below.
• If relevant, please include the hls4ml project files, which were created directly before and/or after the bug.

Quick summary

Using the newest container in PR #1200, we get the following errors in test_sr.py that look like version mismatch:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <hls4ml.backends.symbolic.passes.expr_templates.HLSCodePrinter object at 0x7ff50c72afe0>
expr = cos_lut(x3)
    def _print_not_supported(self, expr):
        if self._settings.get('strict', False):
>           raise PrintMethodNotImplementedError("Unsupported by %s: %s" % (str(type(self)), str(type(expr))) + \
                             "\nSet the printer option 'strict' to False in order to generate partially printed code.")
E           sympy.printing.codeprinter.PrintMethodNotImplementedError: Unsupported by <class 'hls4ml.backends.symbolic.passes.expr_templates.HLSCodePrinter'>: cos_lut
E           Set the printer option 'strict' to False in order to generate partially printed code.
/home/hls4ml/miniforge/envs/hls4ml-testing/lib/python3.10/site-packages/sympy/printing/codeprinter.py:582: PrintMethodNotImplementedError
----------------------------- Captured stdout call -----------------------------

[marco66colombo]

I've been investigating this issue, using the container specified in PR #1200.

• Test with sympy==1.10.1: This version corresponds to when the last commit on hls4ml.backends.symbolic.passes.expr_templates.HLSCodePrinter was made. I confirmed that the test runs successfully with this version.
• Test with sympy==1.13.3: The latest version of sympy fails with the same error. It appears there have been changes in sympy that affect how custom functions are handled by the printer.
• Dependency on sympy==1.13.1: Due to dependencies with torch 2.6.0, this version of sympy is installed automatically, but it also results in the same error.

I am currently trying to figure out the exact changes in sympy between versions 1.10.1 and 1.13.1 that might be causing this bug. I'm exploring potential solutions to make HLSCodePrinter compatible with newer versions of sympy. Any insights or suggestions would be greatly appreciated.

[marco66colombo]

Further Investigation on hls4ml and sympy Compatibility Issue

I've delved deeper into the issue and found a potential workaround as well as some insights into the cause of the problem:

Temporary Workaround:

• By setting the strict option to False in the HLSCodePrinter's settings, the immediate error is resolved. This setting bypasses the strict type checking that leads to the PrintMethodNotImplementedError. hls4ml-true/hls4ml/hls4ml/backends/symbolic/passes/expr_templates.py
  

Root Cause Analysis:

• The compatibility issue arises from changes introduced in sympy between versions 1.12.1 and 1.13.1. The change affecting our case is located in the codeprinter.py file.
  Comparing with previous version: sympy commit diff
• Specifically, the setting of strict to True by default leads to failures when sympy encounters unsupported functions.

Possible fixes:

Adjusting the strict setting to False might be a simple fix but could lead to issues, especially with older sympy versions where the strict setting is unrecognized.

I think there are two possible fixes:

• Update the code to make strict=False the default and require a minimum sympy version in pyproject.toml?
• Or, should we limit the sympy version to a maximum threshold (1.12.1) to avoid these issues, keeping the existing code unchanged?

Tagging @vloncar for insights on the best path forward. Any additional feedback would also be highly valuable.

[vloncar]

From casually browsing sympy code it seems to me that strict has existed since 1.12, just the default value changed in 1.13? That particular line got there between 1.12 and 1.12.1, which are ~2 years old by now. I would go with the first solution if possible, setting the minimum version that works (1.12.1)

[marco66colombo]

I tried with 1.12.1, but it shows the same error.
In the sympy releases, I downloaded the .zip file with the code of version 1.12.1, and it has the strict parameter insympy/printing/codeprinter.py, but if I do pip install sympy==1.12.1, and check the code, it doesn't have the script parameter, so it still fails. It seems like a bug in the release of that version.
With version 1.13.1, everything works fine.

[vloncar]

WOW 😳 . That level of amateurism in the release process trumps everything we did for hls4ml releases 😄 .

Thanks for looking into it. We should then fix the dependency to 1.13.1. Does that have any side effects like dependencies on other libraries or python version itself?

[marco66colombo]

Apparently, it should not have other side effects, but I will double check it

[marco66colombo]

It is kind of a different issue, but always related to test_sr.py:

the test test_pysr_luts tries to import pysr, otherwise it skips the test case.
I noticed that in all the GitLab jobs, it is always skipped, since pysr is not available. I tried to install it, but the test case has some errors.
I wanted to ask if fixing this is a priority, since it is usually skipped, or if I should first focus on completing all the other changes and go back to this in the future.

[vloncar]

Fix this one then open another issue for the problems you encounter in pysr_luts test and we'll deal with those there after this one (maybe not immediately).