mumax / 3

GPU-accelerated micromagnetic simulator
Other
457 stars 151 forks source link

Relax() stopping criterion (issue #146). #148

Closed jsampaio closed 6 years ago

jsampaio commented 6 years ago

Code for issue #146 . This changes the relax() stopping condition so that it stops when the maximum torque reaches a user definable threshold ('RelaxTorqueThreshold').

godsic commented 6 years ago

@jsampaio It appears to me this pull request changes Relax() default behavior. Isn't it?

jsampaio commented 6 years ago

@godsic This pull request changes the Relax() current behaviour. As it is, relax() does not stop when the system is relaxed, it stops when the system is steadily evolving, i.e. when the dm/dt is constant. The main change of this patch is changing the stopping condition to a dm/dt approaching zero. I've written more details of the things in the current Relax() that I think should change in issue #146 .

godsic commented 6 years ago

@jsampaio thanks for the clarification. Our merging policy is that any changes, apart from the bug fixes, shouldn't change mumax3 default behavior. So I would suggest you to use either RelaxTorqueThreshold = 0. or RelaxTorqueThreshold = -1. as the default value (whatever value feels more logical to you) that reproduces Relax() current behavior. Then, if desired user can override this by setting RelaxTorqueThreshold to the desired value.

godsic commented 6 years ago

@jsampaio Thanks. I guess we are approaching the point of merging. I basically have three comments:

JeroenMulkers commented 6 years ago

The Relaxation procedure is an essential part of mumax3, so I think it's best not to be too hasty in modifying anything.

Consider the following statement:

Stop the relaxation if the (1) average (2) maximum of the torque (a) doesn't change any more (b) is approximately zero

Basically, we have a '2D binary option space'. In the current version we use option (1a), which should indeed be the default if we add other options in order to ensure BC.

If we add option (2b) as suggested by @jsampaio, I think we should include the other options as well. That being said, I am not yet convinced that adding option (b) is necessary. @jsampaio, do you have a script which clearly demonstrates that (2b) is a better stopping criteria than (1a)?

jsampaio commented 6 years ago

@godsic : I still think that the default (current) behaviour of relax() is not desirable, as it frequently yields results very far away from a relaxed state. Perhaps if you guys agree, in the future, we can change the default RelaxTorqueThreshold so to use the new relax() condition, if we're all satisfied that it poses no new problems.

I have a demonstrator simulation, comparing the old and the new relax, and Minimize() : relaxTriangularTrackSimplified3.txt

In the current code, any value of RelaxTorqueThreshold <=0 will use the default behaviour. I think this is ok, as the threshold for the maximum torque should always be a positive finite number.

I've run the tests on the folder /test. However, I get some errors with both branches of the code, on tests that do not have relax()...

JLeliaert commented 6 years ago

I agree with @jsampaio that the current relax function sometimes does not lead to an energy minimum and an improved version is welcome. Even if we want to keep the default behaviour as it is now, I think we should include the new version with the RelaxTorqueThreshold option.

jsampaio commented 6 years ago

@JeroenMulkers you've summarised the problem very neatly. My opinion is that option (2b) is better than the others, if we want that Relax() drive the system to a local energy minimum. Let me explain what I think is bad with the other options:

There are lots of cases where the current Relax() (with condition 1a) stops too early. I've attached an example in the previous comment: a domain wall in a wedged track.

JeroenMulkers commented 6 years ago

@jsampaio thank you for the script. It will definitely be useful to improve the relax function!

I agree with you that a converging criteria based on the maximum of the torque instead of the average torque might be better in some cases.

I also want to point out that if you have a constant torque, then this constant has to be zero because the magnetization can only make rotations on the unit sphere. This means that it is a good condition to check if the magnetization is in a local energy minimum.

jsampaio commented 6 years ago

@JeroenMulkers : That's only true for the torque vector. In the case of relax(), if there's a constant torque, that constant can be different from zero. It's not the torque vector we test, it's the magnitude of the torque vector. Moreover, we consider either the maximum or the average of the torque over all the cells.

Take the case of a moving domain wall at constant speed. The maximum torque (magnitude) will be constant but correspond to different cells as the DW moves. The average torque (magnitude) will also be constant.

JeroenMulkers commented 6 years ago

I see. It is indeed the magnitude of the torque which is checked and which could be a constant. Thank you for pointing that out!

godsic commented 6 years ago

@jsampaio Would you be so kind to replace lines 85-95 in your most recent relax.go implementation with exact copy of the master's code and I am happy to merge then.