-
Notifications
You must be signed in to change notification settings - Fork 12
Fix z_pos and add automatic port cutting to parametric reactors #610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
UPDATE: With these changes made to the points attributes of these parametric components z_pos translates the port cutters in the expected direction. |
The term "vertical" may also be a bit confusing since some planes are "horizontal". Tricky |
Good point. Maybe 'port_offset' could be suitable? Then in the docstrings it can be made clear that this is the offset from the origin in the direction of the first coordinate of the workplane? |
I wonder if the solution would be to make these ports more generic and ask the user to provide a (float, float) point coordinates in the workplane instead of setting (0, float). This would solve the issue of naming cause we could name this |
I like the sound of this solution, it could solve all of these issues. I shall work on implementation. |
|
z_pos parameter has been replaced by a center_point attribute to allow center of cutter face to be specified in two dimensions |
Change port architecture
|
number_of_coils is difficult to implement correctly alongside port_azimuth_placement_angle as one impacts the other. Therefore, number_of_coils will be removed as a parameter (it may be added back later) |
removed number_of_ports as argument
|
Quite a lot going on here. Wondering if the ports should be in an inherited version of the base ball and submersion reactors as not everyone will want ports. Would that make it any easier to implement? |
Proposed changes
PR which fixes the problem with port positioning. The z_pos parameter has been replaced with a center_point parameter which allows the port cutter faces to be translated in 2 dimensions rather than 1. This ensures placement can be controlled in all workplanes.
PR also attempts to add automatic port cutting to the parametric reactors.
A utility method called perform_port_cutting has been added to the utility.py script to perform this function.
CURRENT STATUS: Works as expected for large ports and small fillet radii.
KNOWN BUGS: For small ports, sometimes not all ports are cut from the outboard blankets and large fillet radii can cause errors.
Types of changes
What types of changes does your code introduce to the Paramak?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
No further comments