Arguments for Rectangle and RectangleMesh are inconsistent
RectangleMesh takes in
RectangleMesh(double x0, double y0, double x1, double y1, ..)
Rectangle takes in
Rectangle(double x0, double x1, double y0, double y1)
Why?
Comments (17)
-
-
reporter After reading it again, I think that is what the Rectangle constructor takes, just labelled very strangely:
/// Create rectangle defined by two opposite corners /// x = (x0, x1) and y = (y0, y1). /// /// *Arguments* /// x0 (double) /// x0-coordinate of first corner. /// x1 (double) /// x1-coordinate of first corner. /// y0 (double) /// y0-coordinate of second corner. /// y1 (double) /// y1-coordinate of second corner. Rectangle(double x0, double x1, double y0, double y1);
When I saw (x0, x1, y0, y1), I thought {x,y} referred to the coordinate axis and {0,1} referred to the vertex. But the Rectangle constructor means it the other way around -- {x,y} refers to the vertex, and {0,1} to the coordinate axis!
-
True! This needs to be fixed. I think having (x0, y0) as the coordinates of vertex 0 is more intuitive. But I'm not sure this needs to be fixed in DOLFIN. This code is anyway being moved into mshr: https://bitbucket.org/benjamik/mshr
-
I suggested in the past that using tuples would be clearer:
mesh = RectangleMesh((x0, y0), (x1, y1))
-
That would be messy in C++.
-
@logg Doing
RectangleMesh mesh(Point(0.0, 0.0), Point(2.0, 1.0));
looks pretty tidy to me.
-
Maybe, but I still like RectangleMesh(x0, y0, x1, y1) better.
-
It's not a scalable solution. The actual interface is
RectangleMesh(x0, y0, x1, y1, n0, n1)
, and in 3D it'sBoxMesh(x0, y0, z0, x1, y1, z1, n0, n1, n2)
. There is plenty of scope for user error. -
If so we should be consistent and insist on Point everywhere in the interface.
-
- changed milestone to 1.6
-
Update: the new version of Rectangle in mshr now has this interface:
Rectangle::Rectangle(dolfin::Point a, dolfin::Point b)
We should do the same in DOLFIN for
RectangleMesh
. -
-
assigned issue to
-
assigned issue to
-
Yes, I like using
dolfin::Point
when it's a point. -
I'll make a
point
of fixing this today. -
- changed status to resolved
Should be fixed now. Both RectangleMesh and BoxMesh now require Point, but IntervalMesh does not (would be consistent but a bit silly).
-
Fix issue
#299: Use Point in RectangleMesh and BoxMesh interface→ <<cset 5f692e51033e>>
-
- removed milestone
Removing milestone: 1.6 (automated comment)
- Log in to comment
I don't know why, but think the first one is more natural (specifying two corners that span the triangle).