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

Add Differential Cross Section interface, Compton and PerturbativeQED Model #18

Closed
wants to merge 0 commits into from

Conversation

AntonReinhard
Copy link
Member

@AntonReinhard AntonReinhard commented Oct 5, 2023

@AntonReinhard
Copy link
Member Author

Currently fails because it depends on QEDjl-project/QEDbase.jl#25

Copy link
Member

@szabo137 szabo137 left a comment

Choose a reason for hiding this comment

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

First comments on the draft.

src/compton/differential_cross_section.jl Outdated Show resolved Hide resolved
in_phase_space::AbstractVector{NumericType},
out_phase_space::AbstractVector{NumericType},
)::Float64 where {NumericType<:QEDbase.AbstractFourMomentum}
if (!isapprox(sum(in_phase_space), sum(out_phase_space); rtol = sqrt(eps())))
Copy link
Member

Choose a reason for hiding this comment

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

Additionally, one must check if the passed momenta are on-shell, i.e. p*p==mass(<respective particle with momentum p>)^2 for all incoming and outgoing particles.

However, we should have two versions of this function: i) an unsafe version, which does not check energy-momentum conservation and on-shell momenta, and ii) a safe version, which checks both.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought the idea was that _differential_cross_section is inherently unsafe and any assertions happen in _assert_valid_input, which is where the on-shell-ness is tested currently.

Copy link
Member

Choose a reason for hiding this comment

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

This is correct, but the check for on-shell momenta is not input validation but a physics, i.e. the differential cross section is zero, if any of the external momenta does not obey p*p=mass(...)^2, similar to the energy-momentum conservation. The word unsafe means just an implementation of this function, which assumes, that the input momenta are all on-shell and conserve momentum and energy. The reason for this is, that it should be possible, to check this somewhere else, and therefore not again in the differential cross-section. For example, RAMBO produces momenta, which are automatically on-shell and energy-momentum conserving, which means these properties must not be checked here again.

src/differential_cross_section.jl Outdated Show resolved Hide resolved
src/differential_cross_section.jl Outdated Show resolved Hide resolved
src/differential_cross_section.jl Outdated Show resolved Hide resolved
src/differential_cross_section.jl Outdated Show resolved Hide resolved
test/compton.jl Outdated Show resolved Hide resolved
test/compton.jl Outdated Show resolved Hide resolved
test/compton.jl Outdated Show resolved Hide resolved
@SimeonEhrig
Copy link
Member

Currently fails because it depends on QEDjl-project/QEDbase.jl#25

You can run the unit tests with a development branch as dependency: https://github.com/QEDjl-project/QED.jl/blob/dev/docs/src/ci.md#unit-tests-for-ci-users

@AntonReinhard AntonReinhard force-pushed the compton branch 3 times, most recently from 86b319a to cc48356 Compare October 13, 2023 13:46
@AntonReinhard AntonReinhard force-pushed the compton branch 3 times, most recently from 163cdb5 to deb0a04 Compare October 17, 2023 15:31
@szabo137 szabo137 mentioned this pull request Oct 18, 2023
1 task
@szabo137 szabo137 added this to the Release-next milestone Oct 24, 2023
AntonReinhard pushed a commit that referenced this pull request Nov 13, 2023
This PR adds a simple implementation of fermion and boson propagators. 

This should be merged *before* #18 because the latter will use the
functionality.

- replace dev-version of `QEDbase` with release `v0.1.5` (cf. QEDjl-project/QEDbase.jl#39)

---------

Co-authored-by: Uwe Hernandez Acosta <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: CompatHelper Julia <[email protected]>
Co-authored-by: Anton Reinhard <[email protected]>
@AntonReinhard
Copy link
Member Author

The docs building step currently fails because referenced types/functions have moved to QEDbase now, I'm not sure how to proceed with that. I don't want to delete the references or put the links to the QEDbase docs in manually.

Maybe MultiDocumenter would handle this?

@SimeonEhrig
Copy link
Member

I don't think it solves the problem. MultiDocumenter compiles only the rendered HTML documentation of different packages.

I found this issue: JuliaDocs/Documenter.jl#688
So it is not official documented.

Maybe we can use as workaround markdown links [foo()](doc.com/api#foo), if we have a API reference page.

Comment on lines 40 to 56
# TODO: replace this with some less computationally intensive way to figure out how many states there are
photon_in_bstate = Vector{SLorentzVector{ComplexF64}}(
base_state(
Photon(), Incoming(), photon_in, _spin_or_pol(process, Photon(), Incoming())
),
)
electron_in_bstate = Vector{BiSpinor}(
base_state(
Electron(),
Incoming(),
electron_in,
_spin_or_pol(process, Electron(), Incoming()),
),
)

# average over incoming polarizations/spins, but sum over outgoing pols/spins
normalization = 1.0 / (length(photon_in_bstate) * length(electron_in_bstate))
Copy link
Member Author

Choose a reason for hiding this comment

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

@szabo137 How should we do this?
Some anonymous or _ function that just maps from AbstractPol/AbstractSpin to 1 or 2?

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be another interface function for scattering processes, e.g. averaging_norm(proc::AbstractScatteringProcess). For the specific implementation, I think a map of the AbstractSpin/AbstractPol to 1 or 2 seems convenient.

Nevertheless, I am currently working on a refactoring of the differential_cross_section function, which includes all internal functions like incident_flux or matrix_element as interface functions on AbstractScatteirngProcess. Maybe we should discuss the internals of the current implementation of differential_cross_section in this context.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah that makes sense. Do you want to make a PR into this branch or how do you plan to do it?

Copy link
Member

Choose a reason for hiding this comment

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

I think differential_cross_section can be disentangled from this one here. Therefore I suggest making a separate PR from dev, and after merging, we can update this one here.

Copy link
Member

Choose a reason for hiding this comment

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

I started the refactoring of differential_cross_section here: #38

This changes the scattering process interface from differential_cross_section to more fine granular functions, i.e. incident_flux, averaging_norm, matrix_element, and phase_space_factor. The actual differential_cross_section is downgraded from an interface function to a default implementation using the new interface. Currently, it's a draft-PR because it is not finished yet and the tests might be broken.

@AntonReinhard
Copy link
Member Author

I don't think it solves the problem. MultiDocumenter compiles only the rendered HTML documentation of different packages.

I found this issue: JuliaDocs/Documenter.jl#688 So it is not official documented.

Maybe we can use as workaround markdown links [foo()](doc.com/api#foo), if we have a API reference page.

This would work, but for example AbstractParticle alone is referenced 8 times already, so I'd have to put that link into lots of places... I guess there's not really a better way right now though. Other than not referencing at all.

@szabo137
Copy link
Member

I have another crossref for the docstring issue: JuliaDocs/Documenter.jl#1343

Another workaround would be, to include the docstring of the at_ref function/type to the documentation of QEDprocesses, e.g. in the api.md page. However, this is a bit hacky, because it means to include QEDbase in the makedocs of QEDprocesses (I can provide a showcase or PR if needed). In this case, at_ref works again without specifying the URL, but it seems to be a very ugly solution.

@AntonReinhard
Copy link
Member Author

I'm sorry, I somehow managed to kill this PR by accident. I saved the commits but I might have to open a new PR.

@AntonReinhard AntonReinhard mentioned this pull request Mar 27, 2024
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.

3 participants