Skip to content

Feature/angles#232

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

Feature/angles#232
godsic merged 4 commits into
mumax: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

godsic commented Jul 5, 2019

Copy link
Copy Markdown
Contributor

@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

godsic commented Jul 5, 2019

Copy link
Copy Markdown
Contributor

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