Skip to content
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

[Test Failure] Swig RunPlan != can't compare with None? #1233

Open
Robadob opened this issue Oct 2, 2024 · 3 comments
Open

[Test Failure] Swig RunPlan != can't compare with None? #1233

Robadob opened this issue Oct 2, 2024 · 3 comments

Comments

@Robadob
Copy link
Member

Robadob commented Oct 2, 2024

Couple of Python test failures (debug/windows) whilst investigating the SWIG 4.2.1 issue, note this is with the Python wheel built with SWIG 4.2.1. Although, I've built it with 4.0.2 and received the same failures.

Not immediately clear their cause, these weren't touched in the recent bugfix so that's a coincidence.

====================================================== FAILURES =======================================================
____________________________________________ TestRunPlan.test_constructor _____________________________________________

self = <test_RunPlan.TestRunPlan testMethod=test_constructor>

    def test_constructor(self):
        # Create a model
        model = pyflamegpu.ModelDescription("test")
        plan = None
        plan = pyflamegpu.RunPlan(model)
>       assert plan != None

simulation\test_RunPlan.py:13:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyflamegpu.pyflamegpu.RunPlan; proxy of <Swig Object of type 'flamegpu::RunPlan *' at 0x0000022AD963CB40> >
rhs = None

    def __ne__(self, rhs):
>       return _pyflamegpu.RunPlan___ne__(self, rhs)
E       ValueError: invalid null reference in method 'RunPlan___ne__', argument 2 of type 'flamegpu::RunPlan const &'

..\..\build\lib\Debug\python\venv\Lib\site-packages\pyflamegpu\pyflamegpu.py:16808: ValueError
_________________________________________ TestRunPlanVector.test_constructor __________________________________________

self = <test_RunPlanVector.TestRunPlanVector testMethod=test_constructor>

    def test_constructor(self):
        # Create a model
        model = pyflamegpu.ModelDescription("test")
        # Declare a pointer
        plans = None
        # Use New
        initialLength = 4
        plans = pyflamegpu.RunPlanVector(model, initialLength)
>       assert plans != None

simulation\test_RunPlanVector.py:26:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyflamegpu.pyflamegpu.RunPlanVector; proxy of <Swig Object of type 'flamegpu::RunPlanVector *' at 0x0000022AD95E5BC0> >
rhs = None

    def __ne__(self, rhs):
>       return _pyflamegpu.RunPlanVector___ne__(self, rhs)
E       ValueError: invalid null reference in method 'RunPlanVector___ne__', argument 2 of type 'flamegpu::RunPlanVector const &'

..\..\build\lib\Debug\python\venv\Lib\site-packages\pyflamegpu\pyflamegpu.py:18426: ValueError
=============================================== short test summary info ===============================================
FAILED simulation/test_RunPlan.py::TestRunPlan::test_constructor - ValueError: invalid null reference in method 'RunPlan___ne__', argument 2 of type 'flamegpu::RunPlan const &'
FAILED simulation/test_RunPlanVector.py::TestRunPlanVector::test_constructor - ValueError: invalid null reference in method 'RunPlanVector___ne__', argument 2 of type 'flamegpu::RunPlanVector co...
================================ 2 failed, 674 passed, 12 skipped in 727.95s (0:12:07) ================================
@Robadob
Copy link
Member Author

Robadob commented Oct 2, 2024

Tbh the actual test looks a bit janky, it should probably just be a test that runplan constructor doesn't throw an exception, this equivalence doesn't really match the new/delete behaviour of the C test.

Though arguably, I'd prefer swig to detect when a class of wrong type or None is passed to equality and always return false. This same error could be present throughout the codebase. Can't find any obvious documentation for how SWIG should handle this edge-case, and not clear in the slightest how it ever didn't occur given I managed to reproduce it with SWIG 4.0.2.

@ptheywood
Copy link
Member

Can confirm this error is occuring under linux too, with Python 3.10 and Swig 4.2.1, however a local build using Swig 4.0.2 passess these tests successfully, implying it might be a change in swig?

From users writing python, it should be possibel to compare to None without haveing to also check the type first, so it is something we should try and resolve so that the test (which has been unchanged in 3 years since it was originally written) passes.

The rc1 wheels from our wheel house which were built with 4.0.2 also still pass the test.

@Robadob
Copy link
Member Author

Robadob commented Oct 27, 2024

Swig 4.3.0

Test passes (skipped test is operator+, test that should fail is constructor).

But there's some new deprecation warnings. (Related SWIG issue swig/swig#2881, though this states it was present in 4.2.x?)

(venv) C:\Users\Robadob\fgpu2\tests\python>py.test simulation/test_runplan.py
================================================= test session starts =================================================
platform win32 -- Python 3.12.3, pytest-8.3.3, pluggy-1.5.0
rootdir: C:\Users\Robadob\fgpu2\tests\python
collected 12 items

simulation\test_RunPlan.py .....s......                                                                          [100%]

================================================== warnings summary ===================================================
<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type SwigPyPacked has no __module__ attribute

<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type SwigPyObject has no __module__ attribute

<frozen importlib._bootstrap>:488
  <frozen importlib._bootstrap>:488: DeprecationWarning: builtin type swigvarlink has no __module__ attribute

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
====================================== 11 passed, 1 skipped, 3 warnings in 0.60s ======================================

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

No branches or pull requests

2 participants