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

DOC: demonstrate expression serialization #291

Merged
merged 14 commits into from
Oct 20, 2023
Merged

Conversation

mmikhasenko
Copy link
Contributor

@mmikhasenko mmikhasenko commented Feb 16, 2023

The first attempt to serialize symbolic model (save-model.ipynb) and reuse in a separate notebook (load-model.ipynb)

Currently, the model is saved as a pickle file that includes:

dict_forms = {
    "intensity_expr": unfolded_intensity_expr,
    "variables": {k: v.doit() for (k, v) in model.variables.items()},
    "parameter_defaults": model.parameter_defaults,
    "sigma3": sigma3via12(),
}

It would be useful to be able to retrieve the symbolic model with minimal dependencies.
Here are some things to discuss:

  • what is a good balance between done/substituted expressions
  • Is it possible to dump customary nodes, like Kibble, BW, ... - minimal library for lineshapes(?)
  • Foresee the use with polarization

@redeboer redeboer added the 📝 Docs Improvements or additions to documentation label Feb 19, 2023
@redeboer redeboer changed the title Save load notebooks DOC: demonstrate expression serialization Feb 19, 2023
@redeboer
Copy link
Member

redeboer commented Feb 19, 2023

Thanks for the PR!
I merged the notebooks and moved them to the appendix. The notebook is a bit like an experimental TR, but I think it's fine to leave it in the documentation repository (if only because it's more convenient with the model formulation).

  • what is a good balance between done/substituted expressions

Probably best addressed through ComPWA/ampform-dpd#44

  • Is it possible to dump customary nodes, like Kibble, BW, ... - minimal library for lineshapes(?)

I think pickle should already be able to handle it, see e.g. ComPWA/ampform#139. You will still need to call doit() after importing though.

  • Foresee the use with polarization

It is indeed complicated of a long-term solution for this: "use" depends on what the requirements of the target application are. This also points to a larger issue: so far, this repository was more like an application (an analysis), not a general-purpose library. We want to generalise the code now, but we have to decide what general functionality this library should provide. See also #224.

@mmikhasenko
Copy link
Contributor Author

@redeboer what is missing here for merging?

@redeboer
Copy link
Member

We could merge it (it works), but the solution presented is copy-paste snippets, because there is no function that does the serialization for you. But we can also let this PR be limited to showing how to serialize so that it is at least in the documentation.

@redeboer redeboer added this pull request to the merge queue Oct 20, 2023
@redeboer redeboer removed this pull request from the merge queue due to the queue being cleared Oct 20, 2023
@redeboer redeboer enabled auto-merge (squash) October 20, 2023 11:41
@redeboer redeboer merged commit 6d6fa14 into main Oct 20, 2023
26 checks passed
@redeboer redeboer deleted the output-symbolic-model branch October 20, 2023 11:44
@mmikhasenko
Copy link
Contributor Author

The current github page docs do not have the section on the model serialization. Is it a matter of rerunning the tox job in CI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📝 Docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants