fix: cog calc#174
Conversation
|
@Krande could you do a code review, change the label to release-auto, and squash and merge if all looks good? I see the codspeed-benchmark test fail at "tests/profiling/test_instancing.py ::error::Error: failed to execute the benchmark process, exit code: 1 Error: Process completed with exit code 1." However, when I run the test locally it seems to be no issues. |
|
@oleandor yes, will do it later today. The codspeed.io error can be disregarded. Will remove that test in an upcoming release |
|
This looks good! I'll finish a more detailed review of it soon. I do believe that there needs to be created a relevant test in |
Krande
left a comment
Krande
left a comment
There was a problem hiding this comment.
Great stuff! I added comments on a few key items. If you want me to implement some of the suggestions I have proposed, it would really help if you can create a good unit-test that captures the updated COG calculations you are applying here end-to-end.
| # abs_place = plate.placement.get_absolute_placement() | ||
| # origin = abs_place.origin | ||
| # for pt in plate.poly.points3d: | ||
| # p_tra = origin + pt.copy() | ||
| # ET.SubElement(polygon, "position", {"x": str(p_tra[0]), "y": str(p_tra[1]), "z": str(p_tra[2])}) |
There was a problem hiding this comment.
comments like these must be removed
|
We might wanna fix the glb export also to be consistent with the genie xml export and cog calc? Cannel and angular profile is not matching. 
|
…one for beams that are set with genie flush option
…januar_cog_calc # Conflicts: # tests/core/api/cog/files/beams_constant_offset.xml # tests/core/api/cog/test_cog_beam_offsets.py
…one for beams that are set with genie flush option
|
ok, so i updated some things and made some tests. they all pass except for one beam, it has the genie flush option (IPROFILE). this beam was made in param model where it exist as a TPROFILE before being exported to gnx. Since genie does not have Tprofiles (only unsymmetrical I profile) - the failing test seem to have something to do with cadit/gxml/read/read_beams.js. If you have time to look into this and the rest of the reorganization you commented on, please let me know. |
|
Yup, have started looking into how the reorganised code should be. Hopefully have it ready tomorrow |
|
I moved the test files to where the rest of the test files are located and also removed the |
…in justification module
|
@oleandor, I have added the OffsetHandler class and implemented the redesign. I also added "get_volume" and "get_mass" functions to the Beam object. The tests are passing. Is there anything else you'd like to add to this PR or should I start the process of merging this? I noticed some todo's. Ideally these should be fixed/removed before merging the PR |
|
Great! Let me do a quick check
|
|
I found some cog calc issues (probably unrelated to the test) that i need to fix before we merge. I'll keep you updated. Its related to equipments and and point mass (solved). And plates (working on it) |
…e cog calculation (not sure what is does but it fixed the cog issue related to para model SPACES that have EQUIPMENTS(!): removed this: and self.placement.is_identity() is False:). So now it is only # Apply plate placement the same way as solid_geom
if self.placement is not None:
|
alright! so i fixed the issues with the cog calc for plates and equipments. i also tested the justification in param models and now it all seems to work as wanted! @Krande could you have a final check on the latest updates, and if it seems good please go ahead with the merge |
…d clean up unused dependencies
…ng in beam offsets test
… flush option) to match output format
|
ok, so i found another issue that i think i have fixed. i will check some more before i push the solution |
…ted to is_identity()). why the curret commit fixes the issue is not understood; however, it fixes it
|
Let me know when you are done. I will fix the failing test and merge this later today if you are finished |
Great! Thx! I'm finished now. |
…an up imports in `read_beams.py`
|
👋 Hi there! I have checked your PR and found no issues. Thanks for your contribution! PR Review:I found no pr-related issues.
Python Review:I found no python-related issues. Python Linting results:
Python Packaging results:
|
2 participants
fix part cog and mass calc and genie xml write so it gives matching results! (issue #173)
added section properties; section centroid Cgy and Cgy (not to be confused with neither Cy, Cz, Shceny nor Shcenz which are shear center properties)
when creating a beam, added parameter to choose if wanting to use the offset flush option in genie when writing xml. if it is set to True it will use the flush option, if not it will set a constant offset. both will be the same in genie, however, if updating to a new section in genie later on, it will will update automatically if the genie flush is enabled.