FLAMEGPU2
FLAMEGPU2 copied to clipboard
SWIG 4.1.x and 4.2.x
@Robadob found that manylinux builds were failing, this appears to be due to a new swig release 4.1.0 8 days ago which has since been incorporated into the manylinux containers used for building (2 days ago).
Currently we require swig 4.0.2 or greater (due to required bugfixes), and 4.1.0 is the first release after this.
We should probably allow either, i.e. keep the current CMake Logic though we have a few potential options too:
- Update regular CI to use 4.1.0 too?
- Keep 4.0.2 as the minimum, but fetch 4.1.0 if swig is not found?
- Or strictly require 4.0.2 and not allow more recent versions
Edit
More recently, swig 4.2.x has been released, which is again causing problems. See comments below.
Swig has had several more releases, including 4.2.1. Currently investigating if this is a cause of errors which do not occur in wheels built from 2024-01-12.
AttributeError: 'MessageSpatial3D_Description' object has no attribute 'newVariableID'. Did you mean: 'newVariableInt'?
A local build using 4.2.1
reproduced the error (and built much quicker than older swigs).
4.2.0 errors during pyflamepgu compilation, as an assertion is encountered within swig. This appears to be related to friend
statements, and was fixed in 4.2.1 by https://github.com/swig/swig/commit/c7ab6a01c6582b92db9328e2f3daa67081f05f6e.
4.2.1 does appear to cause errors, so we need to either:
- Fix our .i file when swig >= 4.2.0 is used.
- Prevent Swig >= 4.2.0 from being used
Given 4.2.0 is known to cause problems, we should probably explicitly block that one regardless? Or atleast add an cmake warning that it'll probably not work (incase someone is using 4.2.0 but with the above patch applied).
Ok I had a very quick play with this.
Changed
%define TEMPLATE_VARIABLE_INSTANTIATE_ID(function, classfunction)
TEMPLATE_VARIABLE_INSTANTIATE(function, classfunction)
%template(function ## ID) classfunction<flamegpu::id_t>;
%enddef
// Array version, passing default 2nd template arg 0
%define TEMPLATE_VARIABLE_ARRAY_INSTANTIATE_ID(function, classfunction)
TEMPLATE_VARIABLE_ARRAY_INSTANTIATE(function, classfunction)
%template(function ## ID) classfunction<flamegpu::id_t, 0>;
%enddef
to
%define TEMPLATE_VARIABLE_INSTANTIATE_ID(function, classfunction)
%template(function ## ID) classfunction<flamegpu::id_t>; // swapped this
TEMPLATE_VARIABLE_INSTANTIATE(function, classfunction) // with this
%enddef
// Array version, passing default 2nd template arg 0
%define TEMPLATE_VARIABLE_ARRAY_INSTANTIATE_ID(function, classfunction)
%template(function ## ID) classfunction<flamegpu::id_t, 0>;
TEMPLATE_VARIABLE_ARRAY_INSTANTIATE(function, classfunction)
%enddef
And it seems to have now generated the ID methods that weren't there before.
Possibly related to this issue, but it doesn't appear any other methods have gone missing, though according to the reply it should warn us if things are being ignored.
https://github.com/swig/swig/issues/2954
Have reported the issue to SWIG: https://github.com/swig/swig/issues/3060
Swig 4.3.0 adds python 3.13 support, so we'll probably want to switch to that for CI atleast and make sure that 4.3.0 works as intended (it appears to, and fixes #1233).
In which case I'm inclined for us to:
- Keep swig 4.0.2 as our minimimum required
- Add a CMake Warning if 4.2.0 is found - stating that it is a bit broken? Maybe even an error.
- Add a CMake warning if 4.2.1 about the
== None
issue?- Replace the 3 occurences of
!= None
withis not None
to let 4.2.1 test suite pass, but still prefer to not have people use 4.2.1 / warn them about comparing to none with ==/!=?
- Replace the 3 occurences of
- (maybe?) Update our swig download if not required version to 4.3.0?
- (maybe?) Update Windows/ubuntu CI builds to use 4.3.0 when we add 3.13 to the build matrices?
- (maybe?) Configure pytest to not emit deprecatation notices, which Swig 4.3.0 causes to appear (but marked for fixing in 4.4.0). See https://github.com/FLAMEGPU/FLAMEGPU2/issues/1233#issuecomment-2440166825
Currently, manylinux still contains swig 4.2.1, but it will likely be updated automatically by an every friday CI workflow at some point soon (whenever 4.3.0 makes it onto pypi, the latest version is 4.2.1.post0
, uploaded on 2024-10-23, which is after the 4.3.0 release date. The pypi package doens't seem to be maintained by the (main) swig devs).
I would have suggested we add a Swig CI workflow which checks different versions of swig behave as intended, but as we don't actaully run any tests in CI due to the absence of GPUs it wouldn't have actually spotted most of these issues, so not worth it.