openmc-dev
Combine class structure of RegularMesh with RectilinearMesh
The RegularMesh class is a more specific version of the RectilinearMesh, and only differs in one way - the elements in direction i (x, y, or z) are all the same width (as opposed to customizable element-to-element widths). Conceptually, their shared base class StructuredMesh should house most of their common functionality to improve code readability and maintenance.
A nice benefit of doing this refactoring is automatically allowing RectilinearMesh to work in 1-D and 2-D. It is currently limited to 3-D, while RegularMesh is not.
I can’t say with certainty why the structure is that way, but my guess would be to leave room for a HexagonalMesh sub-class (or others, perhaps RZ?) that we don’t have yet; such a class would share very little internals with StructuredMesh.
From: April Novak [email protected] Sent: Thursday, October 15, 2020 6:24 PM To: openmc-dev/openmc [email protected] Cc: Subscribed [email protected] Subject: [openmc-dev/openmc] Combine class structure of RegularMesh with RectilinearMesh (#1695)
The RegularMesh class is a more specific version of the RectilinearMesh, and only differs in one way - the elements in direction i (x, y, or z) are all the same width (as opposed to customizable element-to-element widths). Conceptually, their shared base class StructuredMesh should house most of their common functionality to improve code readability and maintenance.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/openmc-dev/openmc/issues/1695, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAH5GM4PZMLY6CBPGXOOPYLSK6ACJANCNFSM4SSTXSAQ.
This proposed change wouldn't preclude adding additional structured mesh-type classes to OpenMC. We would just change the class structured to be something like this:
Mesh
/ \
StructuredMesh UnstructuredMesh
/ \
OrthogonalMesh HexagonalMesh
/ \
RegularMesh RectilinearMesh
The current StructuredMesh class would be renamed to something like OrthogonalMesh, and we'd create a new StructuredMesh that would have the methods that are actually shared by RegularMesh, RectilinearMesh, and HexagonalMesh. This would be quite minimal - renaming classes in the header file and the scoping in the source file, but no changes to the Python API or user experience.
My OOP philosophy is to reduce code duplication - seeing as there's like 300 lines of nearly identical code shared by RegularMesh and RectilinearMesh, I think "merging" their common functionalities into a base class (whether it is StructuredMesh at this moment or a further intermediary like OrthogonalMesh in the future) is a win-win from code readability and maintainability.
That makes perfect sense then; I was viewing this as moving RegularMesh and RectilinearMesh’ shared methods up to StructuredMesh.
Definitely a win-win!
From: April Novak [email protected] Sent: Friday, October 16, 2020 9:08 AM To: openmc-dev/openmc [email protected] Cc: Nelson, Adam Gregory [email protected]; Comment [email protected] Subject: Re: [openmc-dev/openmc] Combine class structure of RegularMesh with RectilinearMesh (#1695)
This proposed change wouldn't preclude adding additional structured mesh-type classes to OpenMC. We would just change the class structured to be something like this:
Mesh
/ \
StructuredMesh UnstructuredMesh
/ \
OrthogonalMesh HexagonalMesh
/ \
RegularMesh RectilinearMesh
The current StructuredMesh class would be renamed to something like OrthogonalMesh, and we'd create a new StructuredMesh that would have the methods that are actually shared by RegularMesh, RectilinearMesh, and HexagonalMesh. This would be quite minimal - renaming classes in the header file and the scoping in the source file, but no changes to the Python API or user experience.
My OOP philosophy is to reduce code duplication - seeing as there's like 300 lines of nearly identical code shared by RegularMesh and RectilinearMesh, I think "merging" their common functionalities into a base class (whether it is StructuredMesh at this moment or a further intermediary like OrthogonalMesh in the future) is a win-win from code readability and maintainability.
— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/openmc-dev/openmc/issues/1695#issuecomment-710069889, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAH5GM353AZFNTISCC6N5NTSLBHTJANCNFSM4SSTXSAQ.