FLAMEGPU / FLAMEGPU2

FLAME GPU 2 is a GPU accelerated agent based modelling framework for CUDA C++ and Python
https://flamegpu.com
MIT License
99 stars 19 forks source link

AgentPython does not capture external variables/constants #1144

Closed Robadob closed 7 months ago

Robadob commented 8 months ago

As raised in #1142, where a user is attempting to port the Sugarscape example to agent python.

The below code does not generate something which can be compiled, as AGENT_START_COUNT's declaration is not included if passed to codegen via the identifier of the agent function.

AGENT_START_COUNT = int(2)

@pyflamegpu.agent_function
def movement_request(message_in: pyflamegpu.MessageArray2D, message_out: pyflamegpu.MessageArray2D):
    foo = AGENT_START_COUNT 
    return pyflamegpu.ALIVE

The agent python boid's example however uses device functions, which do appear to be dynamically pulled in. Hence, the reason a user would expect this to work.


This does however create concern that the included AGENT_START_COUNT would exist as an implicit __device__ variable, which could lead to race conditions if misused by a user. It make make more sense to convert them to either #define or constexpr.

ptheywood commented 8 months ago

The typing module does have the final decorator / annotation from python 3.8 (which in practice doesn't get enforced along with the rest of the type annotations), but we could potentially only support this use-case with those annotations present?

Our min python is now 3.8 since 3.7 went EOL.

This is only useful if ast respects this (which I'm not sure it will, I haven't looked), but if it does might make tracking what variables to include as constants nicer. Getting a nice error message out if it hasn't been used though would still be a tad grim.

https://docs.python.org/3/library/typing.html#typing.Final https://peps.python.org/pep-0591/

Robadob commented 8 months ago

The ast module does have a constant function, but i've no idea what is supposed to trigger it.

    def _Constant(self, t):
        """
        Restrict most types of constant except for numeric types and constant strings
        Picks up some obvious conversions such as None and Bools
        """

Though perhaps it means literal.

ptheywood commented 8 months ago

ast does have atleast some typing support, but doesn't explcicitly mention Final.

https://docs.python.org/3/library/ast.html#type-parameters

So it might be accessible, will probably just have to try and see what it returns.


Edit: With the annotation:

$ python3 -c "import ast; import typing; print(ast.dump(ast.parse('PI: Final = 3.142'), indent=4))"
Module(
    body=[
        AnnAssign(
            target=Name(id='PI', ctx=Store()),
            annotation=Name(id='Final', ctx=Load()),
            value=Constant(value=3.142),
            simple=1)],
    type_ignores=[])

Without:

$ python3 -c "import ast; import typing; print(ast.dump(ast.parse('PI = 3.142'), indent=4))"
Module(
    body=[
        Assign(
            targets=[
                Name(id='PI', ctx=Store())],
            value=Constant(value=3.142))],
    type_ignores=[])

So the annotaiton does get recorded, though the value is marked as consant in both cases (because its a single expression assigning a literal to a variable, might not in a more complex scernario)

Robadob commented 8 months ago

Discussed and agreed solution would be to decorate such statements with a new decorator and pull them in as constants (probably constexpr).

Some debate over whether these values should actually be stored as environment properties.

Robadob commented 8 months ago

Discussed and agreed solution would be to decorate such statements with a new decorator and pull them in as constants (probably constexpr).

Some debate over whether these values should actually be stored as environment properties.

This doesn't appear possible, Python only allows decorators to be attached to functions and classes. So probably returning to Pete's original typing.Final typehint idea.

Robadob commented 8 months ago

Have played with this a bit this morning.

The below code replacing the final statement of __init__.py::translate().

        # Filter constants
        module_annontations = inspect.get_annotations(module) #  requires python 3.10
        module_members = inspect.getmembers(module);
        prepend_c_source = ""
        # Find all annotated variables
        for key, val in module_annontations.items():
            if val.__name__ == "Final":
                # Locate the literal for that variable (Python will precompute anything e.g. math.sqrt(12.5))
                for mem in module_members:
                    if key == mem[0]:
                        prepend_c_source += f"constexpr auto {mem[0]} = {mem[1]};\n"
        return prepend_c_source + codegen(tree)

Will cause

import typing
import math

TEST: typing.Final = 5
TEST2: typing.Final = 12
TEST2: typing.Final = math.sqrt(12 * 36)
TEST3: int = 12

To be translated to

constexpr auto TEST = 5;
constexpr auto TEST2 = 20.784609690826528;

Requires some proper testing, but most of the way there. It appears that Python precompiles global vars before inspect accesses them, so only the name and value are available.

Question is whether we can define our own pyflamegpu.constant attribute/how?

Robadob commented 8 months ago

Question is whether we can define our own pyflamegpu.constant attribute/how?

class foo:
  pass;
TEST3: foo = 12

Makes TEST3s annotation be reported as <class '__main__.foo'> Yet it's value remains 12, so this may work.

ptheywood commented 8 months ago

Question is whether we can define our own pyflamegpu.constant attribute/how?

Do we need to? For an optimised build, anything marked as Final being made constexpr will be optimised out if it isn't used in the kernel by nvrtc, so the worse-case impact will be a slightly longer nvrtc compile time, but relative to our already very very hnigh nvrtc compile time it won't be noticable.

Robadob commented 8 months ago

Do we need to?

Consistency with the other decorators, save users needing to import Final from typing etc.

No reason we can't support both

ptheywood commented 8 months ago

It just seems that using the python 3.8+ standard library feature should be preferable to implementing something additional.

Minimal way to add a custom type would probably be to use the explicit type alias from the typing module in the .i file?

Would need to be more complicated to make it do other things, and again just using the standard python feature seems good like the better option to me.

https://peps.python.org/pep-0613/#specification

Robadob commented 8 months ago

already using inspect.get_attributes() which is python 3.10+. can replace that with a manual call to inspect.get_members(module), then iterate to find the __attributes__ key if necessary.

peps.python.org/pep-0613/#specification

I don't really follow the examples here.

ptheywood commented 8 months ago

already using inspect.get_attributes() which is python 3.10+. can replace that with a manual call to inspect.get_members(module), then iterate to find the attributes key if necessary.

Maintaining 3.8 support would be preferable, as it's still an actively maintained python for another ~11 months. 3.9 goes EOL Ocotober 2025.

We could drop support for older pythons if we need to, but the current total wheel downloads from github artifacts for each python version is:

Python Downloads
3.6 175
3.7 616
3.8 328
3.9 251
3.10 468
3.11 61

This is limited by the wheels we provide, and is probably weighted towards whatever is on colab (currrently 3.10) and doesn't include changes over time, but does show that we've had a lot of <= 3.9 downloads.

Don't think we have any way to get time-series info on which pythons are in use.

Robadob commented 8 months ago

I've already added commit that reverts to inspect.get_members() which the same fn is already using.

PR should be ready now, unless we want to go as far as modifying codegen() to constexpr global vars too when a string is passed in.