ubermag / mumax3c

mumax3 calculator.
https://ubermag.github.io
BSD 3-Clause "New" or "Revised" License
22 stars 7 forks source link

Temperature support (updated) #81

Closed SeregaKR closed 5 months ago

SeregaKR commented 9 months ago

support for simulation at finite temperature

P.S. also added limited support for the simulation type, where it's going while certain conditions are met. P.P.S. RunWhile moved to a separate pull request. Only temperature is left here

SeregaKR commented 9 months ago

When doing simulations for dynamics, it's common to use in mumax3 RunWhile(MaxTorque > ). Tried adding the support for it.

Naturally, you can add whatever condition inside the brackets for RunWhile, but I frankly have no idea how to implement it. The necessary translation of all ubermag syntax / structure / variables to mumax3 script is tremendous, and I doubt that is really needed.

Probably it's better not to import this exact change to the master branch (as it include only MaxTorque support), but is better to write about it in the Docs. Another way is to be able to directly pass the conditions in the mumax3 syntax, like e.g.:

td = mc.TimeTorqueDriver() td.drive(system, condition="MaxTorque > 0.005", verbose=2)

instead of (like it's right now in the commit): td = mc.TimeTorqueDriver() td.drive(system, maxtorque =0.005, verbose=2)

samjrholt commented 9 months ago

@SeregaKR Thank you for your contribution! We like the ideas you have suggested, would it be possible to split this into two pull requests, one for the temperature and one fore the torque? We are happy with the way you have added temperature so we would be keen to support this.

We like the idea of adding support for RunWhile but we will have to think with you about the best way to implement the conditions. We will open an issue on this topic and start to brainstorm with you the best solutions. Your input would be really valuable as we have less experience with RunWhile functionality.

SeregaKR commented 9 months ago

@SeregaKR Thank you for your contribution! We like the ideas you have suggested, would it be possible to split this into two pull requests, one for the temperature and one fore the torque? We are happy with the way you have added temperature so we would be keen to support this.

We like the idea of adding support for RunWhile but we will have to think with you about the best way to implement the conditions. We will open an issue on this topic and start to brainstorm with you the best solutions. Your input would be really valuable as we have less experience with RunWhile functionality.

Separated the pull request as asked. This is now only for temperature

samjrholt commented 9 months ago

@SeregaKR Thank you for your fast response!

Please could you do a few more things prior to merging:

SeregaKR commented 9 months ago
samjrholt commented 9 months ago

@SeregaKR That is great thank you, yes the only test currently should be that temperature is written correctly (and in the correct place to the mx3 file)

SeregaKR commented 9 months ago

@SeregaKR That is great thank you, yes the only test currently should be that temperature is written correctly (and in the correct place to the mx3 file)

I checked the mumax3 docs and it seems that there is no strict requirement where we should put the line with Temp. The most important point is to put it before simulation

samjrholt commented 5 months ago
  • Speaking about tests. The only test necessary is to check if the line Temp=10 was present in the mx3 file or not. The test for timedriver here: timedriver.py already has the check for finite temperature (test_noevolver_nodriver_finite_temperature). Not sure what's more to check.
  • Added changes to changelog
  • Added to contributors

@SeregaKR we were looking to release shortly so I wanted to check the status of the PR.

The code changes in the PR look good. I was just having a look for the corresponding changes in the change log and tests but I couldn't seem to spot them. Please could you point me in their direction and then we can release this!

SeregaKR commented 5 months ago

The code changes in the PR look good. I was just having a look for the corresponding changes in the change log and tests but I couldn't seem to spot them. Please could you point me in their direction and then we can release this!

The only change for the temperature was in mumax3c/scripts/driver.py. I didn't make any changes in the tests, as I thought they were covered already by that one (test_noevolver_nodriver_finite_temperature). My own manual tests and consistent usage shows nothing amiss.

What do you mean by changes in the changelog? Sorry, I'm not that familiar with GitHub. I found the pull request here which already mentions the temperature support: pull request 36. Do I need to change the file here? changelog.rst

samjrholt commented 5 months ago

The code changes in the PR look good. I was just having a look for the corresponding changes in the change log and tests but I couldn't seem to spot them. Please could you point me in their direction and then we can release this!

The only change for the temperature was in mumax3c/scripts/driver.py. I didn't make any changes in the tests, as I thought they were covered already by that one (test_noevolver_nodriver_finite_temperature). My own manual tests and consistent usage shows nothing amiss.

What do you mean by changes in the changelog? Sorry, I'm not that familiar with GitHub. I found the pull request here which already mentions the temperature support: pull request 36. Do I need to change the file here? changelog.rst

No problem at all! So the test_noevolver_nodriver_finite_temperature test is actually only only run with OOMMF and not Mumax3 at the moment. I will edit/add some tests to make sure this is checked in the CI. As for the changelog - we have a changelog on the website and you have linked the correct place to change it! Maybe give a go a quickly adding to it in the changlog branch https://github.com/ubermag/ubermag.github.io/blob/changelog/source/changelog.rst