@nov2oj Hi Joo-Von, could you please add corresponding tests, similar to test/topologicalcharge-skyrmion.mx3, test/topologicalcharge-uniform.mx3 and test/topologicalcharge-vortex.mx3?

@nov2oj Hi Joo-Von, could you please add corresponding tests, similar to test/topologicalcharge-skyrmion.mx3, test/topologicalcharge-uniform.mx3 and test/topologicalcharge-vortex.mx3?

@godsic Done. The tolerance for the vortex case is slightly higher, while for the skyrmion case it is slightly lower.

The charge density depends strongly on the discretization because it is not really a density. This could be confusing. Would it make sense to divide the 'lattice density' by the cell volume to get an actual density?

Sure, we could do that. This would mirror then the densities given by ext_topologicalcharge without changing the total charge reported.

I'll add the cell volumes back in

@nov2oj are those lower tolerances due to the edges?

@nov2oj are those lower tolerances due to the edges?

@godsic Not entirely sure. There are visible differences between the densities obtained with ext_topologicalcharge and ext_topologicalchargelattice both at the core and at the edges. At any rate, the estimates of the total charge improve with both methods as the number of layers decrease.

@nov2oj Sorry for not replying in a while. Is it possible to merge finite and pbc versions into one extension? One can figure out if model is finite or with pbc at runtime either in cuda kernel or in go frontend and hide those options from end-users that will for sure mess them up at some point.

@nov2oj Sorry for not replying in a while. Is it possible to merge finite and pbc versions into one extension? One can figure out if model is finite or with pbc at runtime either in cuda kernel or in go frontend and hide those options from end-users that will for sure mess them up at some point.

I agree that having separate versions may be a little confusing, but it comes down to whether or not we want a local definition of the topological charge density, like the one currently implemented. The 'pbc' version provides a useful definition of the local density, while the 'finite' version only ensures that the total charge is correct (the local density reported is a little arbitrary).

I wrote up some notes concerning my proposed implementation, I'd love to have your thoughts on it. Suggestions for improvement are particularly welcome!

ext_topologicalchargelattice.pdf

@nov2oj thank you for providing the document with information about the implementation.

I think we could merge the topologicalchargelattice feature in the master branch. It will definitely be useful feature for many of our users.

However, I'm a bit hesitant to include also the topologicalchargefinitelattice in the master branch for two reasons:

1. As Mykola mentioned, the user might be confused by having two options which seem to be doing the same thing. Hiding the options for the user and select the 'right' option during runtime might also not be a good solution, because the two kernels implement a different approach (even in absence of boundaries or corners).

2. The triangulation used in topologicalchargefinitelattice is assymetric. The inner triangles could have been oriented differently, which might lead to a slightly different result (let me know if I'm wrong about this).

I made some changes to the topologicalchargelattice kernel in a separate pull request (see #265).

In this alternative implementation, corner cases (litteraly) are specially treated. I think this avoids the need to have a separate kernel (topologicalchargefinitelattice) for non trivial geometries.

@nov2oj , I will merge pull request #265 in the master branch, except if you see any problems with this approach.

@JeroenMulkers I think your solution is an elegant one, it looks like it should take care of the special cases. I agree it's better to have a single implementation for this.

@nov2oj , in mumax3 we have the ability to list references to relevant published documents when a certain feature is used. If you are planning to write a paper in which you mention that you have implemented this in mumax3, then let us know and we will add it as a relevant paper so you can get some credit for this work.

I already added the work of Berg and Lusher in the list of relevant publications (b820912). I also mentioned Berg and Lusher in the documentation strings. This will make it easy for users who switch between ubermag and mumax3 and are looking for this feature.