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

Refac test implementation #146

Merged
merged 24 commits into from
Jan 20, 2025

Conversation

szabo137
Copy link
Member

@szabo137 szabo137 commented Dec 16, 2024

With this PR, I propose a major refactoring of the test implementation, mostly to drop QEDcore.jl as a test dependency. See #144 for details.

TODO (unit tests itself)

  • particle: check only particle properties
  • particle: implement and check propagator
  • particle: implement and check base_state
  • process: moving parts that do not belong to processes (marked in the code by TODO)
  • particle stateful: implement some tests
  • cross section: remove coordinate-based evaluation (is checked in PSP tests)
  • core_compat: move to QEDcore.jl

TODO (test implementation)

  • psp: make TestPhaseSpacePoint(...,moms) type-stable
  • psp: implement proper in- and out-psps
  • particle: implement massless particles
  • psl: implement failing PSL

TODO (general)

  • consider testing everything for (TestMomentum{T}, TestMomentumMutable{T}) where T<:Union{Float16,Float32, Float64} (not just for TestMomentum{Float64})

Final remarks

Solves #144.

Makes #140 easier to achive.

@szabo137
Copy link
Member Author

szabo137 commented Dec 26, 2024

@AntonReinhard This one is almost done. I have a fix for the GPU tests, which I tested for CUDA, and I will push if I tested it locally for AMDGPU as well.

I think most of the rest could be reviewed. Sorry for the long list of changes, but most of it is reshuffling and renaming of things.

Btw: I expect no comment earlier than mid January 😉

@szabo137 szabo137 marked this pull request as ready for review December 29, 2024 20:41
@AntonReinhard AntonReinhard linked an issue Jan 2, 2025 that may be closed by this pull request
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.

Very nice, removing the circular dependency will definitely help the future development.
I've looked through more or less on surface-level since it's kind of a lot 😅

test/test_implementation/TestImplementation.jl Outdated Show resolved Hide resolved
test/test_implementation/momentum/test_impl.jl Outdated Show resolved Hide resolved
test/test_implementation/particles/test_impl.jl Outdated Show resolved Hide resolved
test/test_implementation/phase_space_layout/groundtruth.jl Outdated Show resolved Hide resolved
test/test_implementation/phase_space_layout/test_impl.jl Outdated Show resolved Hide resolved
test/test_implementation/phase_space_layout/test_impl.jl Outdated Show resolved Hide resolved
test/test_implementation/phase_space_point.jl Outdated Show resolved Hide resolved
test/gpu/momentum_map.jl Outdated Show resolved Hide resolved
test/interfaces/coordinate_transforms.jl Outdated Show resolved Hide resolved
Uwe Hernandez Acosta and others added 4 commits January 6, 2025 21:05
Copy link
Member Author

@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.

Thanks for the rapid reply. I changed the code accordingly.

test/test_implementation/phase_space_layout/groundtruth.jl Outdated Show resolved Hide resolved
@szabo137
Copy link
Member Author

@AntonReinhard how do we proceed with this one? From my side, it looks like final. But feel free to add more comments 😊

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.

LGTM
What about the one last unchecked box in the PR description?

@AntonReinhard AntonReinhard merged commit db17618 into QEDjl-project:dev Jan 20, 2025
4 checks passed
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.

Proposal: refac test implementation
2 participants