Moving to v1.0.0 w/ API consistency

[connorferster]

Is your feature request related to a problem? Please describe.
PyNiteFEA continues to develop as a great project and I love using it (and teaching others to use it!) I know at least one engineer who relies on it for production engineering work (awesome!)

As PyNite is not in v1.0.0 yet, there have been some breaking changes to the API recently (part of which I addressed in #194). This is to be expected since the major version is still 0. I am proposing that PyNite could be ready for a v1.0 after doing some final house-keeping updates to the API. Once doing a v1.0 release, this would mean that the names of all classes, functions, parameters, and parameter order would stay fixed until it is decided that the existing API no longer serves the future of the project.

To be clear, additional features can be added to the v1.0.0 release (e.g. new element types, new solvers, new methods, new attributes, etc.) but the default settings would need to remain and the new features would be optional.

(Apologies if you are nodding your head saying, "yes, yes, I know this..."; I am just stating it for clarity)

Describe the solution you'd like
I am proposing to open a PR (off of the DKMQ branch) to perform the following tasks to move towards a consistent API and to facilitate a stable v1.0 release:

• Make all attributes snake case. e.g. model.Members would become model.members, and model.LoadCombos would become model.load_combos
• Make all module names snake case. e.g. import Rendering would become import rendering.
  • The exception to this would be FEModel3D.py which I think should remain import FEModel3D for historical reasons and because I think it just makes sense.
• Add type annotations to all user-facing function parameters and class attributes (or all functions/methods, if so desired)
  • @dinglind brought this up in Type Hints #199, also. They make a good point and I would like to do this also. Type hints improve code completion and inline help prompts.
• Any other suggestions or changes you would like me to make.

Describe alternatives you've considered
We could also do nothing and leave things as they are. Breaking changes currently seem to happen unexpectedly though and, when they happen, it gives the impression that PyNite is "buggy". I think that PyNite is stable enough to consider a v1.0 release and give users confidence going forward that their scripts and models written against v1.x releases will always be able to run. This still allows PyNiteFEA to continue to grow rapidly through the addition of new features as attributes, parameters, functions, and classes can always be added without having to change the names of existing attributes, function, and classes.

Additional context
Moving to a 1.0.0 release can also facilitate improved git branch management techniques. See #201

[bjhowie]

I'll jump in and say that I agree that the package is almost ready for a v1.0 release. I have been using the 1D Member elements in production code for a while now and it has performed flawlessly, though I'll admit it is a little tricky to maintain due to the regular breaking changes. I would like to incorporate the 2D elements (Quads) into production code as well, though it seems they're not quite ready yet.

I think this is a good time to open the discussion around what the API workflow looks like in a v1.0 release. I've addded a few of my own thoughts below:

1. I think the use of sections for Member elements should be enforced. At the moment, the user can initiate a Member element with either a section name or by manually passing A, Iy, Iz & J parameters to the constructor. We're not saving much by passing those parameters directly to the member, and storing all section parameters in a specific section class feels much cleaner. Additionally, the section parameters should only be extracted from the section object at analysis time, not copied over to the Member object at member construction. This allows for section properties to be altered between iterative analysis runs without manually having to update the member section parameters before each run.
2. As above, but for material properties. Material parameters should be kept stored in the material object until analysis time for the reasons outlined above.
3. Does a quad section property class make sense? It would really only need to store the thickness of the quad, but would allow that thickness to be easily changed between analysis runs. I dont feel as strongly about this as for Members.
4. Plate elements to be removed. They dont seem to be all that usefull when the Quad element is much more flexible. I undertand the implementation is simpler and there is likely a performance benefit to that, but they really just clutter up the codebase.
5. There is a disconnect in the way the user interacts with the model before and after analysis. To set up a model, we only interact with the model via the methods of FEModel3D, but then to extract the results we need to interact directly with the Node/Member/Quad objects, which can only be accessed by accessing the FEModel3D instances lookup dictionaries. I dont necessarily know what the right approach here is, but the current one feels a little odd.
6. The Material class is poorly documented. It is also unclear why we need to provide both a shear modulus and a poisson's ratio. Can one not be calculated from the other?
7. It is difficult to extract quad internal forces (shears/moments) at a given node without interrogating the topology of the quad. In my opinion, the user should not need to interact with the (r, s) natural coordinate system of the quad

I understand much of the above would constitute breaking changes, but I want to make sure they're considered before pushing ahead with a v1.0 release.

[SoundsSerious]

I am also quite interested in solidifying the API since I think this library is working great and I think has a good balance of scope and ease of use. I think it might be a good idea to share our wishlists in this threads so we can decide where potential breaks in the interface might occur and design an interface that wont break when these changes are added if not at the 1.0 release.

I have a couple of changes I would like to make and think some of them are significant changes so It would be nice to get those in before 1.0 vs after to prevent breaking changes.

1. I've taken a stab at integrating a common materials definitions between sectionproperties and PyNite, and in general I think maybe this could be a separate library since we essentially double our user-base for support in this area. It would also be cool to have some kind of scrape of matweb to populate a materials database which is outside the scope of PyNite.
   https://github.com/Ottermatics/ottermaticslib/blob/master/ottermatics/eng/solid_materials.py

2. I have an integration between defined 2D member profiles as per PyNite and shapely-defined sections that rely on sectionproperties. This is done with an emphasis on unifying the resultant stress calculation including a prediction of failure for the more computationally intense FEA approach of sectionproperties using a Support Vector Machine interface. This works by calculating the true stress via FEA and adding it to a database when the number of points is below a threshold or when the estimated stress is within the (margin of error x safety factor) to failure. An interesting use of this feature is the ability to quickly determine the structure failure loads via iterative solver vs applying loads and then calculating stresses. This might be useful as a 3rd party library if it doesn't fit in here.
   https://github.com/Ottermatics/ottermaticslib/blob/7b73db99a784d2da21e1cc599c3ea4b09c5b4d6a/ottermatics/eng/geometry.py

3. Using a "KDTree" for node definitions, which replaces identifying duplicate nodes, with the added benefit of searching a region for existing nodes.

4. Saving / Loading Case Results to disk, I've outlined this in Displacement Memory Growth & Purge Option + Disk Caching of Displacements #178 but have only progressed a little bit on this

5. I've added a GPU Solver capability enabled via environmental variable since that would likely be an advanced use case.

6. I've improved meshing for cylinders and annular meshes to align the starting node angle

Changes I'd want to integrate are here:
main...Ottermatics:PyNite:structural_integration

Another interesting Idea would be to create a github-organization to integrate engineering oriented libraries that many brilliant people have put together. A library I am using capytaine receives support funding from the National Science Foundation and NREL, which has accelerated their growth. No reason the same could happen here!

[connorferster]

Thanks for the input. Here are my thoughts:

@bjhowie (not completely in the same order as you have):

1. Introduce a Section class so that a Member definition is populated with connectivity information, Section class, and Material class. Basically five parameters (name, start node, end node, section, material). I think that makes sense. I think that might be on @JWock82's road map, too.
2. QuadSection class for quad sections. From what you are saying, I think that makes sense (even though it seems to be just for name and thickness). It would be consistent with frames and allows for the same behaviour of modifying a section object and have it apply across the model.
3. I don't know if plate elements should be removed. They seem like they might have their place in membrane-type structures (which I believe are included in the design examples). @JWock82, thoughts?
4. Poisson's ratio and shear modulus: Agreed, I always found it odd because it is like duplicated information. Defining one or the other should be sufficient.
5. Force extraction from quads: I think @JWock82 is addressing this in the new DKMQ branch.
6. Model interaction before and after analysis: I very much agree with this. It is an awkward but sensible approach. Perhaps the implementation could be wrapped in a method? model.get_member_results("M1", "shear", "Fy", "Combo 1", n_points=200) (for the raw arrays) or model.get_member_diagram("M1", "deflection", "dy", "Combo 1") (for the matplotlib figure). As long as the implementation stays the same (i.e. people can still access dicts directly) and these methods are simply added to the FEModel3D then this does not need to be breaking change but would be a nice quality of life improvement.

@SoundsSerious:

1. Harmonzing with sectionproperties would be fun but I think is outside the scope of PyNite. PyNite Section objects would basically take results from a sectionproperties analysis. Perhaps there could eventually be a method on the Section class that could accept a sectionproperties.Section object and use it to populate a PyNite Section object? Would not be a breaking API change, just a new feature.
2. Sounds cool! As you suggest though, I think it is a 3rd party library extra since there is so much extra scope there.
3. A KDTree would certainly add a lot of "intelligence" to the nodes. Do you think the Node and FEModel3D API could remain as is and this could simply be an implementation detail under-the-hood? It would create the possibility for new method names to do wonderous new things (non breaking changes) but are there any elements of the existing API that would have to change to accommodate the KDTree?
4. Saving models and results to disk would be a great feature (have been thinking of it myself). It seems we could eventually do that by just creating new method names to handle these capabilities (non-breaking change). Do you see any elements of the existing API that would have to change to accommodate this?
5. GPU solver with an added environment variable. If the variable is simply an added option, this sounds like a non-breaking change.
6. I just did a quick read of changes in your branch. I did not see any breaking API changes (just new optional keyword parameters). The one possible breaking change I did see is adding a __hash__ method to the Node object. I don't believe the Node object is immutable so adding a __hash__ method would be an "incorrect" application of the __hash__ method. Making the Node object immutable certainly would be a breaking change. I don't think this is necessary since we can use the node name as a dict key instead of the actual Node.

In summary, @bjhowie is proposing a series of API changes too be considered. I think they make sense but I take direction from @JWock82 :) @SoundsSerious has some very cool features suggested and I think they can all be implemented without API breaking changes (except the __hash__ thing).

@JWock82, would love to hear your thoughts whenever you have the capacity :)

Thanks all!

[JWock82]

Wow! I appreciate all the insight and interest in Pynite. I'm sorry I've been off the radar for a few weeks as I've been moving my family long distance.

Let me say I agree for the most part. I've been trying to decide when is the right time to do a v1.0.0 release, and I think I'm close. I'm still sorting out a few issues with plates/quads on the DKMQ branch, and I'd like these changes to be part of the v1.0.0 release.

I've generally tried to follow the PEP-8 style guide for python, which uses first letters capitalized for class names. I'm open to other style guide suggestions, but I don't want to disrupt too many users.

Pynite does have a Section class already built in, though it's not mandatory to use at this time. I added this class in preparation for pushover analysis (pushover anlaysis is still a work in progress).

Plates and Quads offer different advantages at the moment, but the DKMQ element will bridge the gap. Plates currently produce better stresses near boundaries, but they have the limitation of being rectangular, and do not consider thick plate behavior properly. They also solve more efficiently. Quads have the advantage of being able to be any quad shape, and of being able to model thick plates. The new DKMQ element will give us the best of both worlds. I'm really close to being done with the DKMQ. The models I'm testing are producing good internal stress results, but for some reason I'm getting strange element corner forces. I think I'm chasing down a typo at this point and when I find it I'll push the DKMQ branch to the main branch and will replace Plates and Quads with just Plates that are based on the DKMQ formulation.

The only disadvantage I see between tying shear modulus to poisson's ratio is that sometimes for non-linear material approximation users may want to adjust one without adjusting the other. We may want to keep them separate. For example, adjusting the shear modulus to account for shear cracking while not affecting the poisson effect for axial loads. This is a pretty far fetched case, especially if the built-in stiffness modification factors kx_mod and ky_mod are used for the plates, but there may be other scenarios where it could be useful to have them separated.

To interact directly with Members/Quads/Plates without using the dictionaries we'd have to build some helper methods as you've noted into the FEModel3D class. Easy enough to do, but not a priority to me personally. There's actually a lot that Pynite can do that is not exposed through the FEModel3D class. I'm developing features faster than I'm making them accessible.

I like the idea of integrating Pynite with other programs, thereby increasing its capabilities and increasing the support pool for both programs.

These are great ideas. Let me get the DKMQ branch sorted out, and then I'll start looking at pull requests. Also, if any of you want to become maintainers, let me know. Pynite's usage has exploded in the last year, and I'm getting a lot more e-mails about it than I have in the past. I'm struggling to keep up with them all. Lot's of great ideas, but only one Craig at this point, and I tend to focus on the features that are most useful to me - I have a killer ShearWall class completed I'm looking forward to sharing in the near future - first I want to make sure it's compatible with the DKMQ element.

[connorferster]

@JWock82 This sounds great. I am going to start submitting incremental PRs to address the following (in this order) to move towards v1.0:

1. Attribute snake_case naming
2. Add method names for extracting forces/results (@bjhowie)
3. Implementing the required use of the Section class for frame elements (to go along with the use of the Material) class (@bjhowie)

My intention is to submit small PRs with clear scope to make it easy for you to review/comment/reject/approve each one. I am going to start working on these Thursday, July 25th in the AM.

If you are up for it, I would be happy to be a maintainer (I have capacity for this now). You have my contact info so we can coordinate this offline if you want.

If you could create a v1.0 branch before Thursday, I will submit my PRs to that branch instead of main. That way, when you are happy with v1.0, you can merge into main and issue a release. :)

[SoundsSerious]

@connorferster @JWock82 great points about the scope of materials and sections, there's alot of possibilities but I think youre right that its better to get solid functionality vs adding every feature under the sun, hence the V1.0 stability imperative :)

Looking through my code for what is ready for integration without breaking anything, I think I can work in a similar pattern to @connorferster to minimize the impending integration hell.

In general I am motivated to improve the meshing capabilities as I've found working with multiple meshes to be frankly a pain in the butt, and actually writing meshing code is even less fun. However having a system that could effortlessly merge meshes, or join-frames into assemblies would be really cool

1. a KDTree impementation already comes with scipy, so given the potential improvements in robust meshing I think its a good canidate for an improvement! I have two branches this could potentially come from (will need to catch up to updates since 2023)

• main...Ottermatics:PyNite:mesh_mods passes current tests and has a method for identifying nodes that are close together using KDTree called determine_proximal_nodes as well as a change to align the cylinder/annulus meshes to a start node.
• main...Ottermatics:PyNite:structural_integration has all the changes discussed so far but the only tests that aren't passing are the shear wall for some reason. I think it may have to do with the rectangular openings that are missing, I'm not familiar with this test suite so giving up for tonight. This has some more convenience methods for searching node spaces, and a very reliable merge_duplicate_nodes that solved some issues I was having with integrating multiple meshes (for a floating wind turbine model).

2. GPU Solver is actually pretty simple with a try-fail import on enabled with an env-var that replaces the numpy/scipy solver for dense / sparse matrices. Milage may vary depending on your GPU and memory speed which is the bottleneck here. main...Ottermatics:PyNite:gpu_solver_addin

Features which I think it might take some thinking about are a redesign for saving/loading results, but would be worth it I think. This kind of also fits with the meshing improvements scope. I think data structures our our friends here, weakref.WeakValueDictionary could be useful for automatically purging data from a global view and/or in conjunction with chainmap to join many dictionary like structures to seem like one large dictionary.

Another idea would be to just hold one set of displacements at a time and develop a strong diskcaching method that can allow a lazy run of results that aren't cashed per some kind of structure-load-hash (might not be impossible with networkx and node/edge types : ). This ultimately would allow much larger models and encourage consistency, python-diskcache has a queue which would allow multiple workers to start on load-combos.

On the topic of materials / sections I dont think there really is a good library for this out there, despite matweb existing. I did find this library but it only has a few materials defined https://github.com/mvernacc/material-properties-interchange

@connorferster for snake case and formatting I think maybe we just want to get a CI workflow setup with github actions, using a formatter like black. I've been looking for good boiler plate here to format and push changes via github actions but I've found that a bit unreliable. I think its a big benefit since we spend less time formatting and more time thinking / solving problems :)

[JWock82]

Checking in again... I'm still stuck on these DKMQ elements. They are producing correct internal forces in the plates, correct deformed shapes, displacements and contour plots, but for some reason the statics checks are failing and the plate node forces are not correct. Really confusing to me how some results can be so accurate while others are off. I've stared at the code for hours trying to find the issue and cross-compared with the equations in the literature I'm using to derive the DKMQ, but no luck finding the bug. I think I'm going to keep working at it, but I don't want the DKMQ element to hold up production anymore. This weekend I'm going to look at it one more time. If I don't find the issue this weekend I'll have you start pushing your 1.0 changes to the main branch instead of the DKMQ branch, with the plan that DKMQ will come along later.

[SoundsSerious]

@JWock82 sounds good! I am excited to get into the pyvista changes from that branch (I dont actually know what a DKMQ element is :) ). It should be easy enough to point our PRs to the new branch, feel free to merge these wherever is convenient for your evaluation

@connorferster Thinking about your concerns about immutability on __hash__,:

I found organizing nodes via their hash directly in a dictionary to be useful, hence the change. At present the dictionary structure is organized with the key as each nodes name so a hash of each nodes name and use the node directly as a key in the dictionary is essentially equivalent.

As an alternative in the current api there isn't a way to move nodes so a hash on its X,Y,Z coordinates (vs the name as it is now) would be consistent with current concepts.

In the structural_integration branch before Adding a node I look to see if there is an existing node within a globally defined tolerance (or a custom one), using this it would be possible to create a global list of unique nodes with a positionally defined hash in a way that is safe.

That being said, maybe node names are just another thing to maintain arent they? I could see the benefits of querying by a name however, and as well querying in space.

An alternative could be to allow for node aliases to be queried, or alternatively nodes could be given a readable hash name:

• 3 hex characters is 4096 possible nodes
• 3 alphabetical characters is 17576 possible nodes
• 3 alpha-numeric characters is 46656 possible nodes

[connorferster]

@JWock82 I want to voice my support of you completing the DKMQ branch. This is something you have been working on for some time and would be a real boon to the package. I am excited for the possibilities it brings to thick concrete slab design. If it takes longer to work out the bugs, that's ok with me. @SoundsSerious and I have also forked the DKMQ branch for our PRs so it would be easiest if the DKMQ branch gets merged :)

When I am debugging, I often find the following mantra helpful: "The data that I think I have is probably not the data that I actually have." Meaning, that most of my functions might actually be working but the data that is coming in to them might not be what I am expecting it to be. So that's where I start checking.

You got this!!! Let me know if you want another set of eyes on it (you would need to email me the reference material).

[SoundsSerious]

@connorferster agreed, there's no reason we have to rush this, in fact its probably better if we dont :)

Since it sounds like youre pretty close on DMKQ perhaps we can create a branch off your latest commit on that branch @JWock82 for v1.0 so @connorferster and I can work on the PR merge. Depending on progress & timing DMKQ could be merged to main first then V1 or V1 could be merged to main pulling in your latest changes on DMKQ. The old divide and conquer!

[JWock82]

I’ve tinkered some more with the DKMQ element. It looks like the DKMQ is derived using a swapped nomenclature from MITC4 for rotations about the local x and y axes. Not sure if I’m looking at it right, but I’ll try swapping it and testing a few things. Progress happens one baby step at a time.

[JWock82]

Update: I've isolated the issue and now my statics checks are passing. After checking and rechecking my DKMQ code multiple times, I decided either 2 authors had incorrect DKMQ derivations, or the element wasn't plugging into Pynite the way Pynite expected it to. Apparently, the papers I'm using to derive the DKMQ use a different sign convention from Pynite's sign convention and from the MITC4's and other plate elements' more traditional sign convention. The DKMQ also swaps the x and y axis for bending. I've got good results coming out of the elements now, and just need to swap the sign convention for a few more methods. Isolating that bug stumped me for 2 or 3 months. So frustrating to debug, but so satisfying once the overall statics checks finally passed!

[connorferster]

@JWock82 Great work! 🤩

[bjhowie]

Brilliant stuff, @JWock82! I'd have rage-quit long ago.
We all appreciate your effort.

[JWock82]

Another update: I've done a little more work on the DKMQ element, and more testing, and it is performing nicely so far. I'm noticing a major improvement in plate bending stresses at fixed edges over the old MITC4 formulation. I've still got a few more parts of the code to correct and a few more tests to run, but I think this element will be ready to go within the next week or two, depending on how much free time I can find to finish it up.

[JWock82]

The DKMQ branch has been merged into the main branch.

[connorferster]

@JWock82 Amazing! Congratulations!

[bjhowie]

Following on from the discussions above, I'm going to start working on a pull request to enforce the use of member section properties. This will probably be the most painful of the breaking changes, so best to get it out of the way early.

[bjhowie]

On the topic of breaking API changes, I think it is worthwhile modifying the FEModel3D constructor functions to return the instance of the newly created object rather than the name assigned to it. For instance, the add_node method would return the newly created node instance instead of the node name.

I find myself constantly looking up newly created objects in the relevent FEModel3D dictionaries immediately after calling the constructor methods, which feels a little inefficient.

For consistancy and ease of use, constructor methods that currently accept object names should also accept instances of the relevant object. For instance the add_member method would accept either the names of the end nodes or the node instances themselves, same with sections and materials. This wouldn't be a breaking change so is less critical, but I think it would make sense to implement this along with the above change.

Would be keen to hear other's thoughts on making this change prior to a v1.0.0 release.