amuse icon indicating copy to clipboard operation
amuse copied to clipboard

Work to enable interfaces with different-than-standard index lengths

Open rieder opened this issue 1 year ago • 4 comments

Work for issue #1077

  • [x] Change gd.py to a module for flexibility
  • [x] Create a "GravitationalDynamics64Interface" that ~inherits from "GravitationalDynamicsInterface" but~ changes the indexes to 64 bit?

rieder avatar Oct 14 '24 09:10 rieder

@LourensVeen I think this solution works. I wish it could be done with less code repetition, but I don't immediately see how...

rieder avatar Oct 21 '24 14:10 rieder

As a point of terminology: you converted the gd.py module to a gd/ package. A submodule is a git thing that is nice in theory but a pain in practice, and also package here should not be confused with the wheels or tarballs (or, a long time ago, eggs, except that they're still around somewhat too) that you upload to PyPI, which are also packages but of a different kind. Python...

(This comment brought to you by having spent the day wading through the amuse.rfi code which is horribly inconsistently named and frustrating to work with, sorry for taking it out on your PR :grimacing:)

I'm not sure the subdirectory is all that necessary actually, to clean up amuse.community.interface I would start with moving it to amuse.interface and moving the example.py to the documentation. That would leave six files, which would become eight, which is no big deal at all. If there's a split, I'd do it by field, but then we'd probably get amuse.interface and omuse.interface and such.

Copy-pasting and modifying is ugly, but I didn't find a different solution either, and if there is one it may well be worse. I'd like to replace the whole mess that is amuse.rfi with a new implementation and if I end up doing that, then I may be able to design in a better solution for this. Meanwhile it may have to do, unfortunately.

LourensVeen avatar Oct 21 '24 15:10 LourensVeen

ok to merge, or are changes needed?

rieder avatar Oct 31 '24 15:10 rieder

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 28 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 30 '24 16:12 stale[bot]

I completely forgot I had worked on this, but may still be worth merging?

rieder avatar Aug 01 '25 10:08 rieder

Looking at this again, I think it may actually be possible to still share the code, by effectively emulating a C++ template.

We'd make a base interface like

class GravitationalDynamicsInterfaceTemplate(common.CommonCodeInterface):
    _INDEX_TYPE = None

    # contents here, using _INDEX_TYPE instead of "int32"

And then we could define

class GravitationalDynamicsInterface(GravitationalDynamicsInterfaceTemplate):
    _INDEX_TYPE = "int32"

and

class GravitationalDynamics64Interface(GravitationalDynamicsInterfaceTemplate):
    _INDEX_TYPE = "int64"

That is clean type-wise, as the two interfaces don't inherit from each other, and the template would be effectively an abstract base class. Actually, it should probably inherit from abc.ABC because of that.

Not sure if it actually works, but I could code it up and see if it seems worth it?

LourensVeen avatar Sep 02 '25 13:09 LourensVeen