BOUT-dev icon indicating copy to clipboard operation
BOUT-dev copied to clipboard

Remove `Mesh::load`

Open ZedThree opened this issue 1 month ago • 4 comments

Mesh should be entirely created in constructor

This is a little bit experimental -- the reason we have Mesh::load in the first place is to break the circular dependency between Mesh and Coordinates, and the reason we construct Coordinates in BoutMesh::BoutMesh is to ensure any communication required in Coordinates is properly synchronised. So we might just end up replacing calls to Mesh::load with calls to Mesh::getCoordinates, but at least the Mesh constructor will be atomic

ZedThree avatar Nov 24 '25 11:11 ZedThree

Ah, clang-tidy has spotted a bunch of virtual functions that will need qualifying with BoutMesh::

Yeah, and some tests fail, so this trivial change is definitely not enough. Shame

ZedThree avatar Nov 24 '25 11:11 ZedThree

Ok the more fundamental problem is that we may need to read some quite important variables (nx, for instance) from Options, which involves parsing a string, which involves creating the FieldFactory singleton, which requires a Mesh* -- and we have no way of passing a Mesh* through Options to get to the FieldFactory. Actually, we have no way of creating the FieldFactory singleton on anything but bout::globals::mesh either.

I suppose we could have something like FieldFactory::setSingletonMesh() to set this in the BoutMesh constructor, but that does feel a little gross

ZedThree avatar Dec 02 '25 14:12 ZedThree

Ok the more fundamental problem is that we may need to read some quite important variables (nx, for instance) from Options, which involves parsing a string, which involves creating the FieldFactory singleton, which requires a Mesh* -- and we have no way of passing a Mesh* through Options to get to the FieldFactory. Actually, we have no way of creating the FieldFactory singleton on anything but bout::globals::mesh either.

I suppose we could have something like FieldFactory::setSingletonMesh() to set this in the BoutMesh constructor, but that does feel a little gross

Perhaps we could remove the Mesh* argument to the FieldFactory constructor. It sets a default Mesh*, overridden by optional Mesh* arguments to create2D and create3D methods. Those arguments would now be required.

bendudson avatar Dec 02 '25 17:12 bendudson

Perhaps we could remove the Mesh* argument to the FieldFactory constructor. It sets a default Mesh*, overridden by optional Mesh* arguments to create2D and create3D methods. Those arguments would now be required.

That could work, I think we'd also need to add a Mesh* argument to Options::as (at least for the field versions?)

EDIT: I have added the static FieldFactory::setMesh(Mesh*) here and it seems to work -- the tests all pass at least! I think this experiment mostly highlights where we have circular dependencies between classes

ZedThree avatar Dec 02 '25 17:12 ZedThree