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: add preview of AmpForm-DPD model deserialization #47

Merged
merged 17 commits into from
May 28, 2024

Conversation

redeboer
Copy link
Collaborator

@redeboer redeboer commented May 27, 2024

Added a page for Lc to pKpi that illustrates ComPWA/ampform-dpd#132 (preview).

Warning

The model validation does not work yet, for now the page just illustrates the symbolic rendering of the deserialized model. Keep an eye on ComPWA/ampform-dpd#133 for upcoming fixes.

@redeboer redeboer added the 📝 Docs Improvements or additions to documentation label May 27, 2024
@redeboer redeboer requested a review from mmikhasenko May 27, 2024 12:35
@redeboer redeboer self-assigned this May 27, 2024
@redeboer
Copy link
Collaborator Author

@mmikhasenko the page has been converted from a Jupyter notebook with MyST syntax and is not yet optimal for Quarto. On the road today, hope to get back to that later today. The PR should be merged before tomorrow afternoon.

@mmikhasenko
Copy link
Collaborator

Looks great, @redeboer.
I could not spot any problems right away: maybe it matches once the nan is solved.

@mmikhasenko
Copy link
Collaborator

Combination of s (GeV^2), and masses (GeV) does not look right, if the order of the argument changes (production vs decay of resonance)

image

@mmikhasenko
Copy link
Collaborator

p^{2L} is missing.

image

my form-factor includes z^{2l} in the numerator, that is why there is no p^{2l+1} in julia code. Just 2p/sqrt{s}

@redeboer
Copy link
Collaborator Author

maybe it matches once the nan is solved.

For that it would be easiest to get a section with checksums for dynamics. But indeed perhaps the points you pointed out might fix it already.

@redeboer
Copy link
Collaborator Author

I'm doubting btw between merging this PR for now to get at least the preview up or addressing the issues first. Don't think I can handle that quickly because it might require a new prerelease of AmpForm-DPD.

@redeboer
Copy link
Collaborator Author

Combination of s (GeV^2), and masses (GeV) does not look right, if the order of the argument changes (production vs decay of resonance)

Good point, though more of a design problem + latex rendering problem? You would just have to square the mass. But yeah if there is some argument swapping going on programmatically this is tricky...

@redeboer redeboer changed the title DOC: add preview of model deserialization with AmpForm-DPD DOC: add preview of AmpForm-DPD model deserialization May 28, 2024
@redeboer
Copy link
Collaborator Author

@mmikhasenko seeing that CI takes some time, I would prefer to merge this one (and #45?) asap. Remaining issues will be addressed through ComPWA/ampform-dpd#133 and a follow-up PR here.

@redeboer redeboer enabled auto-merge (squash) May 28, 2024 10:29
@redeboer redeboer merged commit b1831bc into RUB-EP1:main May 28, 2024
3 checks passed
@redeboer redeboer deleted the ampform-dpd branch May 28, 2024 10:31
@redeboer
Copy link
Collaborator Author

Ah sorry, hadn't noticed that the requirement for an approval was switched off... so it automerged already

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.

2 participants