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

refactor!: remove Model interface #357

Merged
merged 12 commits into from
Nov 30, 2021
Merged

refactor!: remove Model interface #357

merged 12 commits into from
Nov 30, 2021

Conversation

redeboer
Copy link
Member

@redeboer redeboer commented Nov 30, 2021

This PR removes the Model interface and related implementations. The interface was introduced with the idea to implement different types of expression trees later on (see ADR-001), but this seems to be a premature optimisation. For now, the Function interface suffices: Function instances can be created through specific functions.

Some major changes:

  • The Model interface and its implementation SympyModel have been removed
  • tensorwaves.model has been renamed to tensorwaves.function
  • There is no way to use performance_optimize() (for now—see Re-implement performance optimization #358)

This means that the workflow becomes:

# helicity_model: ampform.helicity.HelicityModel
function = create_parametrized_function(
    expression=helicity_model.expression.doit(),
    parameters=helicity_model.parameter_defaults,
    backend="numpy",
)

instead of

model = SympyModel(
    expression=helicity_model.expression.doit(),
    parameters=helicity_model.parameter_defaults,
    max_complexity=max_complexity,
)
function = LambdifiedFunction(model, backend="numpy")

This code was not tested or explained in a notebook and with the
upcoming interface changes, the code for performance optimization
(caching) has to be completely reconsidered anyway.
@redeboer redeboer added the ⚠️ Interface Breaking changes to the API label Nov 30, 2021
@redeboer redeboer added this to the 0.4.0 milestone Nov 30, 2021
@redeboer redeboer self-assigned this Nov 30, 2021
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@redeboer redeboer enabled auto-merge (squash) November 30, 2021 20:22
@redeboer redeboer merged commit b527d80 into main Nov 30, 2021
@redeboer redeboer deleted the remove-Model branch November 30, 2021 20:29
@redeboer redeboer changed the title refactor!: remove Model interface refactor!: remove Model interface Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ Interface Breaking changes to the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant