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

initial version of a VelocityBoltzmann with centre of mass shift #11

Merged
merged 4 commits into from
Dec 13, 2023

Conversation

reinimaurer1
Copy link
Member

Adds an optional keyword argument so that VelocityBoltzmann can be set with a center of mass velocity. This is useful for atomic projectiles and low-dimensional models. (Diatomics are already dealt with within the DiatomicQuantised initial conditions)

@clbox is this functionality that you might have already implemented in your yet-to-be-merged edits for atomic scattering?

still playing around with the behavior. Currently, I have only tested the 1D case. Looking for a quick 3D case to make sure it works. Any ideas?

Copy link
Member

@Alexsp32 Alexsp32 left a comment

Choose a reason for hiding this comment

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

I've shortened the centre definition as James suggested and added the capability to modify Distribution centres with + and - after generating them as well.

e.g.

dist_3d=VelocityBoltzmann(300, fill(2000, (3,4)), (3,4))
centre_of_mass_translation=[1,0,0] # centre of mass velocity vector
shift_matrix=repeat(centre_of_mass_translation; outer=[1,4]) #repeat translation vector for each atom
shifted_dist=dist_3d+shift_matrix #returns as if centre were provided on generating Boltzmann distribution

@reinimaurer1
Copy link
Member Author

I like the behaviour that you implemented @Alexsp32 ! Looking at this, I don't think that my optional keyword is even needed anymore and I can revert my edits to VelocityBoltzmann distribution. Would you agree?


Add a permanent translation to a Velocity distribution.
"""
function Base.:+(dist::Normal, translation::Number)
Copy link
Member

Choose a reason for hiding this comment

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

It's probably better to define a new function for this operation instead of overloading +. This is what they call "type piracy" where you implement methods for functions that you don't own on types that you don't own.

Copy link
Member

Choose a reason for hiding this comment

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

Good point. In that case, I'd just remove that function without offering an alternative, since it's easier to re-initialise a single normal distribution with a new $\mu$ than to do it for an entire Matrix.

@reinimaurer1
Copy link
Member Author

this version should do for now. It certainly does what it is meant to do.

@reinimaurer1 reinimaurer1 marked this pull request as ready for review December 13, 2023 12:49
@reinimaurer1 reinimaurer1 removed the request for review from clbox December 13, 2023 12:50
@reinimaurer1 reinimaurer1 merged commit dbeca25 into main Dec 13, 2023
@Alexsp32 Alexsp32 deleted the velocity-com branch February 7, 2024 11:32
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.

More initial conditions Read parameters for VelocityBoltzmann from NQCDynamics.jl simulation.
3 participants