You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@rieder is working on adding Phantom, and Phantom uses 64-bit particle ids. In the GravitationalDynamicsInterface, the new_particle() method has an index_of_the_particle parameter of type int32, and that's what all the other codes use. How do we deal with this?
We discussed this a bit directly, and I'm making this issue to document some key points from that discussion.
First, from type theory, if an interface specifies a 32-bit argument to a function, then an implementation that accepts 64-bit ints is valid because it will accept all 32-bit values too, assuming that there's some automated conversion. But returning a 64-bit value for a function that specifies a 32-bit return value is not allowed, because you may return value that are outside of the range that the interface specifies. Particle ids are passed both ways, so if we just give Phantom 64-bit ids, then we're not properly implementing the GravitationalDynamics interface.
The indexes are in principle internal to a code, they're the index into the worker's internal array of particle data. If codes are coupled and particles are shared, then the same particle may have different ids in the different codes. AMUSE ties them together using a key. This is a 64-bit number (randomly generated by default) that is hopefully globally unique (but see #1076). These are used by the user, e.g. in amuse.couple.ParallelStellarEvolution.
So, it seems (formulating carefully here, because I'm not so familiar with this part of the code yet), that we should be able to do a 64-bit version of the GravitationalDynamicsInterface that uses 64-bit indexes, and the same 64-bit keys for interoperability with everything else. Alternatives could be to update the interface and all the codes as well, but that's a lot of work anyhow and would still need an extension to convert between the 64-bit indices on the Python side and the 32-bit ones on the native side. Unless we also update the native side, which would be even more expensive.
Of course, the GravitationalDynamicsInterface class is more than 900 lines of code. We do not want to copy-paste it. A class variable with the index type won't work, because the functions use the @legacy_function decorator and have no access to the class they're in. So how to do that? Use a custom @legacy_function that takes a reference to the class we're in and substitutes that class variable into the LegacyFunctionSpecification? Or passes it to the decorated function? Does that even work?
The text was updated successfully, but these errors were encountered:
@rieder is working on adding Phantom, and Phantom uses 64-bit particle ids. In the GravitationalDynamicsInterface, the
new_particle()
method has anindex_of_the_particle
parameter of typeint32
, and that's what all the other codes use. How do we deal with this?We discussed this a bit directly, and I'm making this issue to document some key points from that discussion.
First, from type theory, if an interface specifies a 32-bit argument to a function, then an implementation that accepts 64-bit ints is valid because it will accept all 32-bit values too, assuming that there's some automated conversion. But returning a 64-bit value for a function that specifies a 32-bit return value is not allowed, because you may return value that are outside of the range that the interface specifies. Particle ids are passed both ways, so if we just give Phantom 64-bit ids, then we're not properly implementing the GravitationalDynamics interface.
The indexes are in principle internal to a code, they're the index into the worker's internal array of particle data. If codes are coupled and particles are shared, then the same particle may have different ids in the different codes. AMUSE ties them together using a key. This is a 64-bit number (randomly generated by default) that is hopefully globally unique (but see #1076). These are used by the user, e.g. in
amuse.couple.ParallelStellarEvolution
.So, it seems (formulating carefully here, because I'm not so familiar with this part of the code yet), that we should be able to do a 64-bit version of the GravitationalDynamicsInterface that uses 64-bit indexes, and the same 64-bit keys for interoperability with everything else. Alternatives could be to update the interface and all the codes as well, but that's a lot of work anyhow and would still need an extension to convert between the 64-bit indices on the Python side and the 32-bit ones on the native side. Unless we also update the native side, which would be even more expensive.
Of course, the GravitationalDynamicsInterface class is more than 900 lines of code. We do not want to copy-paste it. A class variable with the index type won't work, because the functions use the @legacy_function decorator and have no access to the class they're in. So how to do that? Use a custom @legacy_function that takes a reference to the class we're in and substitutes that class variable into the LegacyFunctionSpecification? Or passes it to the decorated function? Does that even work?
The text was updated successfully, but these errors were encountered: