-
Notifications
You must be signed in to change notification settings - Fork 32
MeshSizeFunction: eval? #49
Comments
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'? |
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? |
Yes, maybe oracle is a bit too esoteric.
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.
The method |
I see now. So it either (current state)
or
I think I'd indeed prefer the second variant because it's more common. A If you expect to read more data from the file, I'd probably make it a function that return several things, e.g.,
or even one object that has attributes
The arguments to |
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.
I was thinking of improving to something like this:
where and then
This folds the intermediate In this case, What do you think about that? |
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 Also, can it be that points, cells = generate_mesh(domain, cell_size, **kwargs) where |
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? |
Indeed. I always think of what is easiest to sell to the average user. Saying "
One could also think about enforcing a "SeismicMesh.domain"-type object, i.e., something that has an
I probably wouldn't put that much logic in |
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. |
Yea, I think that's good advice. I'll likely move the logic out of the generator into |
BTW, for the original class So far, I've made some progress on |
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. |
Yes. Anyway, I think calling it In the meantime, I'm also consolidating the tests into
|
The name
MeshSizeFunction
implies that it's an object that can be evaluated. I don't see aneval()
method or something like that. Am I missing something?The text was updated successfully, but these errors were encountered: