pymad/cpymad

cpymad 1.0 API

coldfix opened this issue · 10 comments

I hereby open a discussion about ideas towards a reworked cpymad API.

I have the following suggestions so far:

  • drop LookupDict and attribute lookups of data in tables in favor of only item access. The pythonic container for data lookup is dict. Thus, I think it is more useful to provide a dict like interface for deferred data like TableColumns and in other places just use dict.
  • remove most parameters from Madx.twiss, .aperture, .survey: fname is not needed anymore, since we are using tables. For the same reason, I think, pattern will not have any influence for the returned data either (other than in jpymad). This means none of the SELECTion parameters isn't required either (?). But then ultimately, these things should be handled manually, if desired.
  • compose Model of objects of Sequence, Range, Optic classes. This will (a) make the internal data members easier (b) allow to edit models in-place more easily by just adding a Sequence instance for example.
  • move computation methods like Model.twiss to Sequence/Range classes. IMO, r.twiss() is much nicer than calling m.twiss(sequence=..., range=...) and provides a more side-effect free interface.
  • make CPymadService only a model locator/loader, rather than an instance manager. I can't imagine many places where the instance manager is needed, since you will always want to store specific Model instances (maybe you have already done some custom MAD-X calls with it)

Of course, these are incompatible with regard to jpymad. Then again, the currently shared interface is so minimal that it raises the question whether it is worth conserving (IMO, no). And even in the shared interface, many parameters are only implemented in one of the backends, have different names in both backends or just behave differently.

Not sure why you want to remove the parameters like pattern, fname etc? pattern is useful in mad-x, so why would it not be useful here? Likewise, fname is useful if you want to store the tfs file for later use by e.g. a separate plotting script (this is quite common).

The LookupDict was created as a common table object for jpymad/cpymad. It could be simplified.

Sequence, Range, Optic classes under Model sounds like a good idea. Similar for the Model.twiss (which is a side-effect of the Mad-X structure itself).

The CpymadService is not really needed by cpymad? I think it was more there because of jpymad compatibility, because this one does need a service handler.

My thought is to remove the parameter fname only from Madx.twiss (etc) signature, but still allowing it in the **kwargs. So, you could still do m.twiss(file=...). Therefore, removing it actually only changes its name to be the same as in MAD-X (which will be simpler to remember anyway).

I think, if you need SELECT, you can just call it separately. That is one additional line and simplifiies the signature of the Madx.twiss, etc methods. I have never actually used it, so maybe, I didn't understand it correctly. I understood it as follows: you can use pattern to select which elements show up in your file. The problem with this is, that it won't affect which elements show up in the memory table (?), so it won't have any effect on the return value of the function, which is read from memory, rather than filesystem. But, having pattern as a parameter to Madx.twiss looks like it would make a difference, even if file is unspecified. Especially, when comparing to jpymad, where pattern is implemented to have effect on the return value, I imagine.

The alternative is to implement pattern matching within cpymad and use it to filter the return value.

I guess, all of this is true for range as well (?). So, maybe implementing it inside cpymad is really the way to go.

The problem with LookupDict is that its methods share the namespace with its data. Currently, there is only .keys() which makes it look like some sort of dict. But if you try to access .items() or similar, you might get a weird error. I'd rather have a true dict-like interface, which every python programmer is familiar with.

So, you have no general objections to evolve the API?

Last question first, no general objections. The code is not yet used by many, so backwards compatibility is not yet a huge issue. It is a good time to improve on the API.

OK, now I follow. Yes, if it is allowed to do by kwargs then it is fine to change fname to file (and a good idea I think). But pattern is done separately right? One of the ideas for me was to not have to do all the separate command calls as you do in Mad-X when calling e.g. twiss (use,...;select, clear; select,...;twiss,...;). Pattern and columns are used internally by Mad-X, and values not needed are not calculated elsewhere (speeding up the calculation). This is not equivalent to a filter afterwards (though the output is).

Ok, that sounds valid. It seems like pattern/range has more side-effects than I imagined, so this is definitely a big argument for keeping these parameters together with the calculation method.

In this case, the returned data should be filtered by cpymad. I will open a separate issue for that, if it is not handled correctly at the time (which I think it is not).

If you SELECT some columns, are the other columns still calculated and updated in the table?

I think they are yes, at least the "important" ones. I don't remember precisely how Mad-X works internally always, I just know that there are lots of tricks to increase computing speed.

I would also propose to change to semantic versioning. There is a nice and short summary what this means on that page.

Okay, I created a branch cpymad-1.0 onto which pull-requests for major API changes can be based. There will probably be quite a lot of new PRs so we can decide about/improve on each change in isolation.

Hey,

I had some more thought and I am now considering to fork the project altogether for now in order to create a more lightweight version that better suits my needs. This way, I can more flexibly develop and try new features while not worrying about CERN requirements. Later on, when I am stable, we can still think about merging both libraries and create a new cpymad version from it. Let me know, if you have any major concerns.

Best regards,
Thomas

Hi,

That might very well be a good idea for you. I don't have a lot of time to follow up on the changes, and I don't think there are a lot of other users out there yet. Hence API breakage is not really that critical.

No objections from my side. Please provide a link to your fork if possible (for my curiousity).

Hey,

my fork currently lives on https://github.com/hibtc/cpymad in the lightweight-fork master branch. This is not completely finished (especially documentation is not updated), but at least my major goals for the new model API have been achieved.

I am wondering whether I should rename the package / namespace, since I am not affiliated with CERN and I don't want to use their name. (Done, to be thorough and safe).