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

Coordinate-based evaluation of differential cross-sections and probabilities #39

Merged
merged 28 commits into from
May 3, 2024

Conversation

szabo137
Copy link
Member

@szabo137 szabo137 commented Nov 30, 2023

Problem description

Currently, differential cross-sections and probabilities can only computed on given momenta. However, in some cases, one wants to calculate those quantities directly on given phase-space parameterizations, i.e. independent coordinates.

Suggested solution

One might use multiple dispatch on the element type of the incoming and outgoing phase-space of differential_cross_section and probability to determine if coordinates or momenta are passed in. Furthermore, it seems sufficient to use the following implementation of differential_cross_section as the actual interface function

The coordinate-based implementation of the differential cross-section should then fall back onto the momentum-based implementation by a phase-space generator, e.g.

generate_momenta(
     in_phasespace_def,
     in_phasespace::AbstractVecOrMat{T},
     out_phasespace_def,
     out_phasespace::AbstractVecOrMat{T}
     ) where {T<:Real}

which returns the respective vector or matrix of SFourMomentum. Using that, the default implementation of the differential_cross_section reads

function _differential_cross_section(
    proc, model,
    in_phasespace_def,
    in_phasespace::AbstractVecOrMat{T},
    out_phasespace_def,
    out_phasespace:: AbstractVecOrMat{T}
    ) where {T<:Real}
    in_moms, out_moms = generate_momenta(proc, model, in_phasespace_def, out_phasespace_def, in_phasespace, out_phasespace)
    return _differential_cross_section(proc, model, in_phasespace_def, out_phasespace_def, in_moms, out_moms)
end

which works generically.

For the evaluation of cross sections and probabilities on sets of phase space points, the
implementation is the same for momenta and coordinates. Therefore, this PR introduces a general phase space element type: AbstractPhaseSpacePoint = Union{Real, AbstractFourMomentum}.

TODO

  • Write docs for added versions of the exported functions.
  • Write an issue on the phase space interface including the momentum generation above.

@szabo137 szabo137 changed the title Coordinate-based evaluation of differential cross-sections and probabilities [WIP] Coordinate-based evaluation of differential cross-sections and probabilities Nov 30, 2023
@szabo137 szabo137 mentioned this pull request Feb 22, 2024
4 tasks
@szabo137 szabo137 requested a review from steindev March 12, 2024 18:25
@szabo137 szabo137 requested a review from tjungni March 12, 2024 18:25
@szabo137 szabo137 marked this pull request as ready for review March 12, 2024 18:25
@szabo137 szabo137 changed the title [WIP] Coordinate-based evaluation of differential cross-sections and probabilities Coordinate-based evaluation of differential cross-sections and probabilities Mar 12, 2024
@szabo137
Copy link
Member Author

@steindev @tjungni If I did not forget something, this PR is ready from my side. I would appreciate some comments from your side. 🙆‍♂️

tjungni

This comment was marked as duplicate.

Copy link
Member

@AntonReinhard AntonReinhard left a comment

Choose a reason for hiding this comment

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

This seems like a lot of code duplication for not too much gain. If I understand correctly, the idea is to allow a new, different type of representation of the FourMomenta as input. So why not make the existing functions generically accept any AbstractPhasespaceElement and then a function (or multiple functions) that convert an AbstractPhasespaceElement into a specific representation?
The interface of this function could even completely generically allow to specify a target type, so if more types might be added later, the only thing that needs to be added are the relevant conversion function implementations.
This function would then be the identity function if it's already the correct representation, and otherwise do whatever it needs to do. This would be the _generate_incoming_momenta() I believe.
Also, it might make sense to have this function only work on a single element and then broadcast where necessary, seems more flexible.

src/interfaces/process_interface.jl Show resolved Hide resolved
src/cross_sections.jl Outdated Show resolved Hide resolved
src/cross_sections.jl Show resolved Hide resolved
@szabo137 szabo137 mentioned this pull request Mar 27, 2024
@szabo137
Copy link
Member Author

szabo137 commented Mar 27, 2024

This seems like a lot of code duplication for not too much gain. If I understand correctly, the idea is to allow a new, different type of representation of the FourMomenta as input. So why not make the existing functions generically accept any AbstractPhasespaceElement and then a function (or multiple functions) that convert an AbstractPhasespaceElement into a specific representation? The interface of this function could even completely generically allow to specify a target type, so if more types might be added later, the only thing that needs to be added are the relevant conversion function implementations. This function would then be the identity function if it's already the correct representation, and otherwise do whatever it needs to do. This would be the _generate_incoming_momenta() I believe. Also, it might make sense to have this function only work on a single element and then broadcast where necessary, seems more flexible.

As we discussed offline, I think this is a great idea, which would avoid a lot of code duplication. However, I think it would be necessary to develop a full-fledged conversion system first, where any given phase-space element type can be converted into four-momenta as the default phase-space element type. Therefore I suggest proceeding with this PR as it is, but open an issue to address your idea. I think adding the phase-space conversion system later will not break the API, which means it should be fine to add it later.

Edit: The respective issue is #49

@szabo137
Copy link
Member Author

@AntonReinhard could you please check is something is left over here? If not, we could proceed with this one 😊

@szabo137 szabo137 merged commit f6bc45e into QEDjl-project:dev May 3, 2024
4 checks passed
@szabo137 szabo137 added this to the Release-next milestone May 3, 2024
@szabo137 szabo137 added the 06 - Feature-request Missing a feature or functionality label Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
06 - Feature-request Missing a feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants