Feature improvements for the Model class

[max3-2]

I think the model class can get some extended features. As @John-Hennig stated this should be kept as general as possible, especially since class inheritance allows easy extension. I will just start by listing some features I use often and add some small explanation and either +0- depending on my thoughts if they should be implemented.

1. Inspection with tags: Currently inspection returns the names of objects. Since most of the advanced features are references by tags, I think those should return a list of tuples with ‘(name, tag)’ +
2. Description of parameters: Adding / editing this field should be supported +
3. Interpolation functions: This seems to be a common way to send data to COMSOL. Addding and editing those should be enabled +
4. Toggling boundary conditions: This can be useful (on/off) but since then will be under a physics node may be hard to generalize 0
5. Generating surfaces from selections: This can help automate exports. Is fairly easy to generalize but I do not know how often this is needed from the API. 0
6. Adding exports: This is Handy but due to the need to specify the variables this can get very specific. Or we allow an API with ‘dict’ which easily creates exceptions I guess -
7. Tracking loaded files: This should be very useful for working with many models to reduce slow IO especially when working with remote servers. I suggest initial tracking by file name and tag, maybe add via a class attribute ‘dict’ +

I can help implement some / most of the stuff. But I will sit for your input @John-Hennig before...

[john-hen]

Thanks for your input. Here are my first thoughts:

1. (tags) Tags are hidden by the entire Python API, that's intentional. I go on quite a "rant" about tags in the tutorial. I think they shouldn't exist, not even in the Comsol API. Obviously they do, so there is no way around them when implementing features. But that doesn't mean they have to be exposed. Also, changing the return value of existing methods is a breaking API change.

2. (parameter descriptions) Agreed. I think this can be done by adding an optional keyword argument to the parameter(...) method. Like parameter('d', description='distance') alters the description, but doesn't change its value, unless a value is also passed. On that note: The parameter() method should also have a way of returning the "evaluated" value, i.e. not the expression as entered (like '1[mm]'), but also its numerical value (1e-6). Just so the caller doesn't have to parse units. I've needed and used that before, but haven't gone back and changed the API method.

3. (interpolation functions) Yes, absolutely. There is already an API method for that: load(file, interpolation). But I've only ever used it to assign image data to an interpolation function. So have never imported data from text files or anything. There is more than one kind of interpolation function in Comsol too, so I don't think they are all covered. We should go over that, look at use cases (what kind of data goes in, which type of function is it assigned to) to then decide what the best API call would look like. Also, that method does not have a great name. But import is a reserved identifier in Python, which is why it's called load.

4. (toggling boundary conditions) I can see how that can be useful. So maybe a signature like deactivate(physics_interface, feature_name) or something like that.

5. (surfaces from selections) I don't quite get what you mean by that. An example would help. I do use advanced selection features quite a bit when I set up Comsol models. Like collecting domains, then get to their adjacent boundaries, maybe do union/difference operations, to then use those for boundary conditions and plots. And when the geometry is complex, many of those operations can be automated using Comsol alone. What part of that needs to be scripted?

6. (exports) There is the export() method. The way I've used it is, I usually set up the models so that any plot I want exported is defined in an Export node (at the very bottom of the model tree). So then an evaluation script would just perform the export on any node that exists, possibly with a custom (model-dependent) file name. If the user wants more results exported, they just define more Export nodes. It may only support image exports (that's what I've used), and not text data. But other than that, what's missing here?

7. (tracking loaded files) Clarification needed. Or an example.

So maybe start with 2., 3., possibly 4. Any help is welcome.

By the way, I've noticed that your name doesn't appear as a "contributor" on the repo's front page. I think it's because the email address in your git configuration does not match the one you use on GitHub. Or something like that. Was just wondering if I did something wrong when I merged your earlier PRs. But seems like it's up to you to fix that if you want to.

[max3-2]

(1) I did never realizes that names / labels have to be unique. If that's the case then scrap tags altogether at API level. However I suggest the following: There are often calls like

        names   = self.exports()
        tags    = [str(tag) for tag in self.java.result().export().tags()]
        tag     = tags[names.index(node)]

to extract tags. Sometimes they have no error handling (like this), sometimes they have (ValueError for _datasets(), guess it should be IndexError?). This could be wrapped into _getTag(group, name) or something similar for consistency.

(3) I'll see what I can come up with. Some remarks: There are different functions which are completely different (Analytical, Interpolation) in their nature and their arguments. I suggest different API functions for each. Also, there are some args which can be set, like reinterpolation or further options. This could be done via **kwargs in python - is this something you are ok with or should they be exposed by args with default values?

(4) My thought exactly, however I would just make that toggle(..., value)

(5) Forget that, too specific I think. If you create an export, you need to specify a dataset. This one would create a dataset from a selection. However the selection needs to be in COMSOL, so creating the dataset using the UI is easier anyway, especially since there are many different types of datasets.

(6) Before exporting, you need to create the export. Or you can change it by changing the target file (already enabled) or the variables. Since this is again very specific, I do not think it is important.

(7) Call load() with the same file twice and it will load twice. If one tracks the files which are loaded, this would be much quicker by returning the already loaded model. Currently, there's only model.names() which does not track the file path itself...

PS: I had a different email attached to the pushes. I added this one to git too.

[max3-2]

I did put together an idea of how functions, in this case an interpolation, could be managed. Since this is more like a blueprint to discuss, I will add it here and not create a PR at this point. I added comments with my thoughts on this...

# This allows a general edit using kwargs
def function_edit(self, function, **kwargs):
    """Edits any node of a function when the kwargs match"""
    # Get the function tags
    for tag in self.java.func().tags():
      if str(self.java.func(tag).label()) == function:
          break
    else:
      error = f'Function "{function}" does not exist.'
      logger.critical(error)
      raise LookupError(error)

    # Edit with typecast
    for keyword, value in kwargs:
        # This should catch possible types, see Reference Guide.
        # Strings seem to be converted on the fly
        if isinstance(value, int):
            value = JInt(value)
        elif isinstance(value, float):
            value = JDouble(value)
        elif isinstance(value, bool):
            value = JBool(value)

        try:
            self.java.func(tag).set(keyword, value)
        except Exception:  # Too general - what is the correct type here
            error = f'Can not set function property {keyword}'
            logger.warning(error)
            # Raise exception?

# This would be the same as already implemented with a slight rename and arg resort
def function_interpolation_load(self, interpolation, file):
    """
    Loads an external `file` and assigns its data to the named
    `interpolation` function.
    """
    for tag in self.java.func().tags():
      if str(self.java.func(tag).label()) == interpolation:
          break
    else:
      error = f'Interpolation function "{interpolation}" does not exist.'
      logger.critical(error)
      raise LookupError(error)

    file = Path(file)
    logger.info(f'Loading external data from file "{file.name}".')
    self.java.func(tag).discardData()
    self.java.func(tag).set('filename', f'{file}')
    self.java.func(tag).importData()
    logger.info('Finished loading external data.')

# This creates the interpolation function. The user would need to check the
# Comsol reference guide for valid kwargs, but the API is fairly compact
def function_interpolation_create(self, name, file, **kwargs):
    """Creates a new interpolation to use down the road"""
    tag = f'int_{randint(0, 100)}'  # This is probably a bad idea since it can create duplicates
    self.java.func().create(tag, 'Interpolation')
    self.java.func(tag).label(label)
    self.java.func(tag).set('source', 'file')
    self.function_interpolation_load(file, name)

    # Those are conservative defaults. They can be overwritted using kwargs
    # or we remove them from here anyway
    self.java.func(tag).set('extrap', 'const')
    self.java.func(tag).set('interp', 'neighbor')

    if kwargs:
        self.function_edit(name, **kwargs)

    return name

[john-hen]

Yeah, let's discuss. Figuring out the best API is difficult. Naming things is hard. I often go back an forth. Though once you have a good concept for the API, the implementation almost writes itself.

On that note… I will probably once again rename some things that are now in master, but not yet released. Because, second thoughts. It happens. But my point is, API design should not be taken lightly.

Just so that we're on the same page, the current "release schedule", if there is such a thing, looks kind of like this:

• Add some useful API features, like the ones discussed here and in related PRs.
• Settle on a robust solution for starting a local Comsol session. I'm thinking to use stand-alone mode on Windows (because it's fast to start up and proven to be stable) and go with client-server mode on Linux/macOS (because of the limitations there), but hide it all behind the convenience function mph.start().
• That's release 0.9.
• Then document and test the use case of multiple workers, either started via subprocess or via multiprocessing. So that on, say, a 64-core machine, which is not unheard of these days, it's easy to spin up as many worker processes. I think that's the killer feature of MPh, really. That's where Python outshines Matlab and Java. But I have my doubts that the client-server mode (on Linux/macOS) would be sufficiently robust to get all these processes up and running, based on my experiences with Matlab. (In stand-alone mode, and thus on Windows, this works like a charm, as I already know.)
• That's release 1.0.
• More "feature releases" (1.1 etc.) can follow. Bug fixes (e.g. 1.0.2) in between.

Regarding API design… In the documentation I talk about the scope limitation of this library. That's not a firmly delineated line, we certainly can push further than what we have now. But the reason I mentioned it, is that the basic design of the implementation (I happened to choose) is not suitable to expose all aspects of the Comsol API. That's because the class hierarchy is flat: It's just the Model class, so all functionality is crammed into a single name space.

Obviously this could be changed. But it would not just be a lot of work, it's also kind of redundant. Because ultimately it would just mirror the class hierarchy of the Comsol Java API. It might add a little bit of flair here and there, like hiding tags and using names instead, but probably not enough to justify such an effort. Plus, it would either break the existing API, or it would be difficult to work around what already exists, because all the "good names" in the Model class are already taken.

Thrameos mentioned in issue #7 that JPype offers functionality to customize Java objects with Python functions. That would be an interesting avenue to explore. But so far I haven't looked into it at all. Just something to keep in mind. There's a chance, at least, it might magically solve a lot of problems.

As for this particular idea… function_interpolation_load() is, as you point out, almost exactly the same as load(). So pretty much a duplication, only with reversed argument order. (In load(), the arguments should not be reversed, so as to avoid API breakage, but they could renamed, as they are not keyword arguments. So, for what it's worth, interpolation could become function.)

Then there's function_interpolation_create(). These long names are an indication that a more sophisticated implementation would introduce an object hierarchy instead. Which effectively just replaces the underscores with periods in application code, but would make the library code more manageable.

And function_edit() is basically a "setter" for the "properties" of a node. By the way, code for the corresponding "getter" can be found in the mph.inspect() function.

So, just thinking "out loud" here… In the flat class hierarchy that we have, maybe we should add a Model method like create(category, name), where category is a string that could be "function" or "selection" or so, and name is a feature name as returned by functions() or selections(), etc. And we could have property(category, name, key, value=None), which subsumes getter and setter (depending on whether or not a value is passed) and would allow the user to customize those newly created features to some extent.

This would map strings to Comsol's object hierarchy. It still doesn't cover everything (only properties, not method calls), but it's a step in that direction.

An alternative would be to have access methods such as function(name) which would return the corresponding Java object. I'm not sure that's a good idea, to mix the two worlds like that, but it may be an option, especially if we can figure out how to customize the Java objects in a beneficial way.

(PS: Comsol exposes a uniquetag() method on each of the feature collections, such as func() or selection(), which neatly solves one issue you commented on in your code.)

(PPS: Catching Exception at that point in your code is fine, I think. As long as it's not a naked except clause, which would also, but mistakenly, catch KeyboardInterrupt, there is no need to be more precise. It's gonna be some kind of Java exception, obviously, which are passed through JPype under their original name. But since the try-except clause is so tightly wrapped, there is really no need to differentiate. It should be re-raised as a Python exception, yes. Generally speaking, anything that is due to a programming error by the user should raise an exception. Which one, I don't know. If in doubt, raise RuntimeError. You may add from None to suppress the Java exception in the traceback, or maybe not, to help the user understand what went wrong.)

[max3-2]

Then document and test the use case of multiple workers, either started via subprocess or via multiprocessing. So that on, say, a 64-core machine, which is not unheard of these days, it's easy to spin up as many worker processes. I think that's the killer feature of MPh, really.

I can say that it is possible to have many servers running, which are distinguished by their ports. They are (by default) closed when a worker is finished. If this is easy to wrap in several processes - I don't know.

Obviously this could be changed. But it would not just be a lot of work, it's also kind of redundant

I would suggest you to not got that way. I think - as you also said - this should be the core functionality. COMSOL is, in my opinion, not too clear about which features work with which input data (see my closed issue...there is Eval and there is EvalPoints ...) - rebuilding this with good code and good exception is something that should be left to COMSOL in the future...
And as we also discussed, extending the model class with tailored methods is quite easy.

Coming to the specific code I posted: I think this would be something that is very often used for convenience - so this is why I have suggested it being added, or at least gave an idea of the functionality that I aimed for.

As for this particular idea… function_interpolation_load() is, as you point out, almost exactly the same as load()....

Yes - I suggested to think about a rename - since it will be only applicable to an interpolation. Model.load() somehow shadows Client.load(). Also, there are several points where data can be loaded to a Model so I found the name very general. The arg order is not that important...

Then there's function_interpolation_create(). These long names are an indication that a more sophisticated implementation would introduce an object hierarchy instead.

Agree - see above. But for clarity I opted for a long name.

And function_edit() is basically a "setter" for the "properties" of a node. By the way, code for the corresponding "getter" can be found in the mph.inspect() function.

This could be generalized as a setter for an arbitrary object by name, which would allow a lot of customization with a simple method in python. However there would be some issues that come to my mind:

• Searching all names in the model tree if no type is given can be slow
• kwargs change per type. As it would be with the setup here, the user would have to look them up. Model.function_edit() (or similar) just handles the type conversion.

But coming to

So, just thinking "out loud" here… In the flat class hierarchy that we have, maybe we should add a Model method like create(category, name) ...

This would be the combination of all this. However when creating it might be difficult to find the right location in the model tree. This goes back to a remark up top where I said that COMSOL it not always to clear about that, e.g. a dataset will be created unter .result(), a function under model and I guess there are very many exceptions here.
I have to look into the java customization, thats nothing I have any experience with.

PS: Found the uniquetag now...thats convenient

[max3-2]

Currently, I do not find a good starting point for improvements which will suit the current package setup and I do not have the time to dig into jpype to get a handle on customization.
Please let me know if I can help in the future.

[john-hen]

Yeah, I've been busy too. I do think there are good ideas here though.

The evaluate() method, as you also noticed, could support more dataset types than it does now. The implementation is already quite long, even though I've limited it to only the dataset types that I needed (field solution, global expression, and particles). There are more, like points, cut planes, etc.

I think providing a create() and property() methods, as discussed above, also has merit. As far as I can tell, that would provide the functionality that you proposed to add, but not just for functions, but for all kinds of features. However, if supporting all (what I called) "categories", that would also be a lot of lines of code.

So both of these suggestion would take some time to implement. Not just writing code, also going through the Comsol API documentation and object hierarchy. Though I do think especially the latter is a good starting point, as it's pretty straightforward. It would be pretty much a long if... elif... block for all the feature categories. I mean, if you have time and feel like it. Otherwise I'll get to it eventually, just not sure when.

[john-hen]

@max3-2
With the commits I've just pushed, I think I'm ready to release 0.9.0. Unless you find more bugs. So if you could run the test suite on your system (run_all.py from the tests folder), that would be great. More features in future feature releases...

[john-hen]

@max3-2
So three of the enhancements you proposed here will be in the upcoming feature release: parameter descriptions, toggling physics features, and model caching. For two other suggestions we've discussed I've created separate issues for further discussion and to track progress: #25 and #26. I will close this issue here once 0.9 is released.

[max3-2]

@John-Hennig Allright, nice!

So if you could run the test suite on your system ...

on macOS everything (still) is flawless. On windows it seems to run good however there are some strange issues with jpype and pyenv so the testing procedure was not perfect. However on either py3.7 or py3.8 it worked with a little manual work, so I do not see any issues with the fixes.

[john-hen]

Okay, great, thanks.

I don't use pyenv. For the tests I've now run with Python 3.6, 3.7, and 3.8, I just have separate Python installations on my Windows machine. Create a new issue if you can pinpoint the bug.

Also, I've created and pinned an issue named "chat" for stuff that doesn't really belong anywhere else. Like, if this pyenv thing remains murky, maybe give more details over there. Not sure I can help, but if it's not a clear bug it may help to discuss it first.