search

QEDjl-project / QEDprocesses.jl

[WIP]: QEDprocesses.jl: Modeling of scattering processes for QED.jl
MIT License
1 stars 3 forks source link

cross sections on phase space points #52

Closed szabo137 closed 1 month ago

szabo137 commented 1 month ago

With this PR, we adopt the phase space points from #51 for the probability and cross section interface.

Further changes: we drop the support of different phase space definitions for the in- and out-phase space.

⚠️ This is based on https://github.com/AntonReinhard/QEDprocesses.jl/tree/phase_space_point and should be rebased to dev and considered for merging after #51 is merged.

szabo137 commented 1 month ago

@AntonReinhard I think this is ready for review.

Currently, there is a QEDbase.jl patch in here. I think we should include this in QEDbase.jl, but the release would take a while. What do you think, is it valid to have patch_something.jl files in there, and remove the functionality, after the respective functions are released?

Edit: The respective PR is https://github.com/QEDjl-project/QEDbase.jl/pull/62

AntonReinhard commented 1 month ago

@AntonReinhard I think this is ready for review.

Currently, there is a QEDbase.jl patch in here. I think we should include this in QEDbase.jl, but the release would take a while. What do you think, is it valid to have patch_something.jl files in there, and remove the functionality, after the respective functions are released?

Edit: The respective PR is QEDjl-project/QEDbase.jl#62

I think this is a good idea, since the alternative is either wait until the patch is merged into the dependency and released, or depending on a non-released state of the dependency. I think the former is slow and the latter is ugly. We could maybe make it a rule though to have a reference to an opened issue/PR in the first lines of each patch_something.jl file. That should prevent the issues getting lost.

szabo137 commented 1 month ago

@AntonReinhard I think this is ready for review. Currently, there is a QEDbase.jl patch in here. I think we should include this in QEDbase.jl, but the release would take a while. What do you think, is it valid to have patch_something.jl files in there, and remove the functionality, after the respective functions are released? Edit: The respective PR is QEDjl-project/QEDbase.jl#62

I think this is a good idea, since the alternative is either wait until the patch is merged into the dependency and released, or depending on a non-released state of the dependency. I think the former is slow and the latter is ugly. We could maybe make it a rule though to have a reference to an opened issue/PR in the first lines of each patch_something.jl file. That should prevent the issues getting lost.

I think this is a great idea! I added the respective information in 86beefe.

AntonReinhard commented 1 month ago

The first two commits in this branch are from me and have no total changes, you might wanna remove them from the branch history. But I guess we squash anyways so it's not important.

szabo137 commented 1 month ago

The first two commits in this branch are from me and have no total changes, you might wanna remove them from the branch history. But I guess we squash anyways so it's not important.

Right, since the commits are squashed anyways, this is not an issue.

szabo137 commented 1 month ago

Looks good to me

A general remark though: Some of the test files (cross_sections.jl for example) are getting pretty long and deeply nested. It might be a good idea to move some of the nesting into a directory/file structure, because it's becoming hard to keep track of what level a testset should actually be at. Should probably make that a separate issue though.

I like the idea and opened an issue: https://github.com/QEDjl-project/QEDprocesses.jl/issues/56