MeshSizeFunction: eval? about seismicmesh

[krober10nd]

Yes. Anyway, I think calling it sliver_removal is more precise so I changed that. Making progress.

In the meantime, I'm also consolidating the tests into

1. test_mesher_serial.py
2. test_mesher_parallel.py
3. test_geometry.py
4. test_sizing.py

[krober10nd]

Oh I see your point. 'MeshSizeFunction' just processes the kwargs. Then each sizing function is built/enforced when the user calls the method 'build'.

Maybe should change the name to 'MeshSizeOracle'?

[nschloe]

I've never heard the expression oracle in programming. Is this a common concept? Perhaps factory would be the better choice.

I don't quite understand yet why there has to be an intermediate step, though. Eventually, it'll have to spit out a function, right? Why not return it right away?

[krober10nd]

Yes, maybe oracle is a bit too esoteric. MeshSizeFactory might be better as it builds a sizing function object.

I don't quite understand yet why there has to be an intermediate step, though.

I agree, there doesn't have to be an intermediate step between calling the constructor and building the mesh size function. The same goes for the generator. I can change that as it simplifies the public api.

Eventually, it'll have to spit out a function, right? Why not return it right away?

The method build of the sizing class returns a class object with several attributes including the mesh sizing function, a signed distance function, the bbox and the minimum element size h0. The reason I did it this way is it simplifies the public api to call the mesh generator--the user just has to pass the sizing function object to the mesh generator. If it just returned a function, then the user would have to pass several more arguments to the mesh generator class constructor.

[nschloe]

The reason I did it this way is it simplifies the public api to call the mesh generator

I see now. So it either (current state)

obj = MeshThingy(filename, h0, bbox, andsoforth)
build_mesh(obj)

or

obj = MeshFunction(filename)
build_mesh(obj, h0, bbox, andsoforth)

I think I'd indeed prefer the second variant because it's more common. A build_mesh function is always expected a domain (here: bbox), some cell size (h0), and perhaps more data.

If you expect to read more data from the file, I'd probably make it a function that return several things, e.g.,

obj, bbox = MeshFunction(filename)
build_mesh(obj, h0, bbox, andsoforth)

or even one object that has attributes

bigobj = MeshFunction(filename)
mesh = build_mesh(bigobj.function, h0, bigobj.bbox, andsoforth)

The arguments to build_mesh can probably be brought in a better order.

[krober10nd]

Okay, one thing that needs to be resolved first is if I can indeed get the domain extents from the file.

I agree though that we can expect the user to know that a mesh generator requires some basic information to operate.

obj, bbox = MeshFunction(filename)
build_mesh(obj, h0, bbox, andsoforth)

I was thinking of improving to something like this:

obj, bbox = MeshSizeFunction(filename, **kwargs)

where **kwargs in the above call are the user's sizing options.

and then

points, cells = MeshGenerator(obj, bbox, h0, **kwargs)

This folds the intermediate build steps into the constructor calls.

In this case, obj in the MeshGenerator class contains both the signed distance function and mesh sizing function interpolants as fields, which are later unpacked inside MeshGenerator. In the case of a custom signed distance function, obj would not be passed and instead the user must register their own call-backs for fh and fd.

What do you think about that?

[nschloe]

It should be functions, not classes, so

sizing_function = get_sizing_function_from_segy(filename)  # perhaps return bbox, too, perhaps not
points, cells = generate_mesh(bbox, h0, sizing_function, **kwargs)

and with signed distance function

def domain(x):
    return x[0] ** 2 + x[1] ** 2 + x[2] ** 2 - 1.0

sizing_function = get_sizing_function_from_segy(filename)  # perhaps return bbox, too, perhaps not
points, cells = generate_mesh(signed_distance_function, h0, sizing_function, **kwargs)

Inside generate_mesh, you can check for isinstance(first_argument, tuple) to discriminate between the cases.

Also, can it be that h0 and sizing_function fulfill the same purpose? If yes, we could have

points, cells = generate_mesh(domain, cell_size, **kwargs)

where cell_size (or edge_size, don't know) is either a float or a function that returns a float for each input x.

[krober10nd]

So the sizing function calls private functions I presume based on which arguments are passed? I like this. Then each sizing function can be separated out as an individual function and you avoid a giant amount of setter and getters.

And yes, that makes sense to pull out the distance function into the script. For the standard domain, it's just a box.

For parallel execution with irregular geometries, I turn the distance function into an regular gridded interpolant and then decompose it inside meshgen. Maybe I could make the decomposition of a structured grid across cores into a decorator?

[nschloe]

So the sizing function calls private functions I presume based on which arguments are passed?

Indeed. I always think of what is easiest to sell to the average user. Saying "cell_size is either a float or a function that specifies the cell size at every point" seems pretty understandable to me. The easier, the better.

And yes, that makes sense to pull out the distance function into the script. For the standard domain, it's just a box.

One could also think about enforcing a "SeismicMesh.domain"-type object, i.e., something that has an eval(self, x) method. One could provide a number of standard domains in SeismicMesh, e.g., Rectangle, Disk, Ball and combinations like Union, Intersection etc. dmsh does that, for example. I'll leave it up to you.

For parallel execution with irregular geometries, I turn the distance function into an regular gridded interpolant and then decompose it inside meshgen.

I probably wouldn't put that much logic in generate_mesh. You could just have generate_mesh expect a signed-distance object that has eval, and all the logic happens within that object, be it a Rectangle, a Ball or some other "irregular" geometry.

[krober10nd]

Ok, I almost got through the sizing function rewrite making everything a function and I'm quite happy with how it's going so far. IMO, it's a lot cleaner and easier to read.

Obviously this breaks everything but I don't mind it for now. I'll work through it over the next day or so.

[krober10nd]

I probably wouldn't put that much logic in generate_mesh. You could just have generate_mesh expect a signed-distance object that has eval, and all the logic happens within that object, be it a Rectangle, a Ball or some other "irregular" geometry.

Yea, I think that's good advice. I'll likely move the logic out of the generator into migration to perform the domain decomposition of the sizing function.

[krober10nd]

BTW, for the original class MeshGenerator, I'm working on converting it two functions: generate_mesh() and improve_mesh(). This simplifies the function as it removes a lot of the if logic that was there.

So far, I've made some progress on generate_mesh in #52 . Hopefully will be done shortly with the improve_mesh. In generate_mesh as you can see in that PR, I've converted the script into many short and simple functions that each do only one thing.

[nschloe]

improve_mesh()

This is the sliver removal, right? Anyway, the PR looks good. I particularly like the fact that it removes many more lines than it adds.