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

Feature/angles #232

Merged
merged 4 commits into from
Jul 5, 2019
Merged

Feature/angles #232

merged 4 commits into from
Jul 5, 2019

Conversation

peytondmurray
Copy link
Contributor

I needed some functions which give the magnetization in terms of the usual spherical coordinates θ and ϕ. I also needed to know the in-plane component of the magnetization rxy, so I implemented all three as a single vector field ext_rxyphitheta with components (rxy, ϕ, θ). The field is calculated on the GPU before being transferred back to host memory, so it's pretty quick. Would anyone else find this useful?

@JeroenMulkers
Copy link
Collaborator

Some users might find this an interesting feature. However, I am resistant to merge the current implementation due to two different reasons.

  1. It does not make much sense to put the three variables in a vectorfield quantity. This will lead to a non-intuitive visualization of this 'vectorfield' in the GUI or when using mumax3-convert.

  2. I do not see the point for having both the in-plane component rxy and the angle theta since rxy=sin theta.

In my opinion it is better to define two scalar fields for the magnetization angles phi and theta, and to not define the in-plane magnetization component rxy.

@peytondmurray
Copy link
Contributor Author

I agree. I removed the vector field and implemented the theta and phi scalar fields.

@godsic
Copy link
Contributor

godsic commented Jul 5, 2019

@peytondmurray Looks good to me.
Please

  • run the unit tests and attach the report;
  • please prefix new quantities in engine/angles.go with ext_;
  • please rename engine/angles.go to engine/ext_angles.go.

The ext_ thing is a way to highlight contributed functionality.

@peytondmurray
Copy link
Contributor Author

Okay, I've attached the ext_ prefix to the scalar fields and renamed engine/angles.go to engine/ext_angles.go. Here's the result of running ./test.bash &> test_result.txt.

@godsic godsic merged commit 91fc98f into mumax:master Jul 5, 2019
@godsic
Copy link
Contributor

godsic commented Jul 5, 2019

Cheers!

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