Skip to content

Feature/angles#232

Merged
godsic merged 4 commits intomumax:masterfrom
peytondmurray:feature/angles
Jul 5, 2019
Merged

Feature/angles#232
godsic merged 4 commits intomumax:masterfrom
peytondmurray:feature/angles

Conversation

@peytondmurray
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@godsic
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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