Skip to content

Conversation

@ztangent
Copy link
Member

This PR eliminates the need to call @load_generated_functions in order to make use of a generative function written in the static modeling language. Instead, new @generated GFI methods (simulate, update, etc.) are defined for each static generative function. These GFI definitions are evaluated after the get_ir and get_options definitions returned by Gen.generate_generative_function, preventing world-age issues.

Besides making the static modeling language easier to use, this change prevents incremental precompilation warnings and breakages which were occurring when multiple sub-packages called @load_generated_functions, of the form:

WARNING: Method definition simulate(Gen.StaticIRGenerativeFunction{T, U} where U where T, Tuple)
in module TestSubPkgA at C:\...\Gen.jl\src\static_ir\simulate.jl:86 overwritten in module TestSubPkgB
on the same line (check for duplicate calls to `include`).
  ** incremental compilation may be fatally broken for this module **

Minimal working example: TestPkg.zip

This warning occurs because @generated function simulate(...) was being defined multiple times:

  1. Once when some dependencyTestSubPkgA that calls @load_generated_functions is precompiled.
  2. Again where another dependency TestSubPkgB that calls @load_generated_functions is precompiled.

The second definition overwrites the first definition for the exact same @generated method, leading to precompilation warnings. This PR addresses the issue by defining a different @generated method for each new generative function.

In addition to the main change:

  • All tests have been updated to remove calls to load_generated_functions or @load_generated_functions.
  • Deprecation warnings have been added to the docstrings and function bodies of those methods
  • Documentation has been updated to reflect that calling @load_generated_functions is no longer necessary.

For documentation and docstrings, we may want to specify a version number, e.g. Gen v0.4.6, for when load_generated_functions is no longer necessary. But apart from that, I'm pretty sure this is safe for the rest of the Gen ecosystem!

@ztangent ztangent requested review from alex-lew and femtomc June 15, 2022 14:29
Copy link
Contributor

@femtomc femtomc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! A nice straightforward fix!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants