[BC] Bore and sym != 1

Question

[BC] Bore and sym != 1

Opened this issue 3 years ago · 31 comments

SebGue commented 3 years ago

Hello @BonneelP

could you tell my why the bore shape is only regarded if sym == 1 (see Lamination.get_bore_desc line 33).

OT: Big sorry for having not finished 'electrical' yet.

Best regards, Sebastian

Answer 1 · 2022-01-20T12:44:07.000Z

Hello Sebastian,

It's because issue #24 is not closed yet:

The method to split the Bore surfaces is not implemented/tested yet (although it won't be long to do with the latest development)
There is also the question about the comp_periodicity of Bore shape but the idea was to use a user defined property (or a dedicated method for BoreFlower for instance)
Last but not least: a dxf import feature can be added by reusing DXF_Surf and a dedicated GUI can be added in LamShape step

No problem for the electrical work, do you need any help on the topic ? Do you think you will have some time to work on the topic ? We are currently focusing on some new Manatee features then we will work on the GUI (cf #487) and SCIM/LUT. We will also have to update all the tutorials and the website now that SciDataTool 2.0 is available :)

Best regards,
Pierre

Answer 2 · 2022-01-24T16:27:20.000Z

Okay, thank you for clarification on that issue. So I may have a look on #24.

For the ELUT thing, I may only have some questions now and then.

BR Sebastian

Answer 3 · 2022-03-24T14:00:01.000Z

Hello @BonneelP

I had some issue with notches on sym. lines as well. Is this also related to this issue?

Best regards, Sebastian

Answer 4 · 2022-03-24T14:03:55.000Z

Hello,

Normally notches and vent are correctly handled by sym. There is a dedicated test in Tests\Validation\Magnetics\test_FEMM_periodicity.py
Maybe it is linked to your notches, can you share your geometry ?

Best regards,
Pierre

Answer 5 · 2022-03-24T16:07:55.000Z

Notch_Test.zip

Answer 6 · 2022-03-24T16:09:25.000Z

Plot is ok, ...

while FEMM is not.

Answer 7 · 2022-03-24T17:15:18.000Z

We first corrected the issue on LamSlot build_geometry or get_bore_line but we forgot that LamHole (thought Lamination) could also have notches. Normally we corrected Lamination build_geometry/get_bore_line recently. I see that there is no "opening surface" in the stator slot. Are you up to date with the latest commit ?

Answer 8 · 2022-03-24T19:44:48.000Z

No, my version is not up to date, my bad.
Now that I tested with latest commits (+ some PR's) there is some other issue, i.e. the boundary lines are not drawn.

While it seems to be drawn before.

Answer 9 · 2022-03-24T20:07:21.000Z

For the notches we add the corresponding surfaces in Lamination.build_geometry:

    # Add the closing surfaces if requested
    if is_circular_radius:
        surf_list.extend(self.get_surfaces_closing(sym=sym))

On your plot all the notches surfaces are missing so the call to get_surfaces_closing may be missing.

I just checked on pyleecan master and it works. I am also impressed to see that it works with the fast draw option :)

Answer 10 · 2022-03-30T07:46:07.000Z

Hello @BonneelP

I started to work on this issue and realized that Lamination.get_bore_line method is unused. Do you think we need this method or can I remove it?

OT: Arc1 and Arc3 got different default 'is_trigo_direction' arguments. Do you mind if I set Arc3's to True.
There is a lot of occurances where the following (multi line) code ...

Arc1(
            begin=Ryoke,
            end=Ryoke * exp(1j * 2 * pi / sym),
            radius=Ryoke,
            is_trigo_direction=True,
        )

could be reduce to this.

Arc1(begin=Ryoke, end=Ryoke * exp(1j * 2 * pi / sym), radius=Ryoke)

Best regards, Sebastion

Answer 11 · 2022-03-30T08:03:26.000Z

Hello,

The idea was to use Lamination.get_bore_line into LamHole build_geometry to handle the notches but if LamHole also have a get_bore_line then indeed it may not be used except if someone needs a real Lamination object with notches.

The default for is_trigo_direction can be changed to have something matching to avoid confusion.
Regarding removing the parameter I prefer not to. Python Zen says "Explicit is better than implicit." and first version of Arc1 didn't had is_trigo and when we realized that it wasn't clear enough (begin, end and abs(radius) can refer to 4 different arcs) it was painful to add all the missing is_trigo ^^" So as a reminder I prefer this parameter to be visible every time to avoid confusion.

Best regards,
Pierre

Answer 12 · 2022-03-30T08:25:59.000Z

Since there is get_bore_desc and get_bore_line, I didn't know what the difference would be. (There is no method description in get_bore_line.) Further only Lamination has the unused method get_bore_line.

... except if someone needs a real Lamination object with notches.

Wouldn't get_bore_desc also return the same list of lines?

Answer 13 · 2022-03-30T08:30:48.000Z

Python Zen says "Explicit is better than implicit." ...

Zen e.g. also says "Beautiful is better than ugly." and "Readability counts." ;-)

Answer 14 · 2022-03-30T08:43:29.000Z

There is another important question. If there is Bore and Notch, how should be handle notches? Place them on the circular bore and cut them with bore or should be move them to onto the Bore surface?

Answer 15 · 2022-03-30T09:08:05.000Z

... sorry for spamming.

I think since there are different approaches how to place notches on Bore, so there should be an additional Notch property controling the placement, e.g.

place on bore (and cut/extend notch to close surface in case)
place on circular bore and cut by actual bore
rotate notch in a way that both end points of the notch line loop are exactly on the bore line

What do you think?

Answer 16 · 2022-04-06T12:23:51.000Z

Hello sorry for the long time to answer.
get_bore_desc and get_bore_line are a sort of workaround/work in progress. Both functions have evolved separately and step by step along the development of the software (adding notches, adding sym...) and I'm not sure that they make fully sense any more. A good clean-up would be great :)

Then I propose is_trigo default to be None to enforce the user to think about what should be drawn ;)

There is another important question. If there is Bore and Notch, how should be handle notches? Place them on the circular bore and cut them with bore or should be move them to onto the Bore surface?

For now we enforce either Bore or Notch and prevent having both. We plan to add the possibility to import Bore radius as DXF then Bore + Notches would be imported and handled as BoreUD (since the real geometry will have handled the "merge part")

Having the possibility to merge Bore object and Notches would be great but I think it may be really too hard to find generic rules that works with all Bore and Notch shape. Then either we can introduce new Bore objects that contains a slot object either we can provide a method to merge with the possibility to select the merge behaviour as an input and use it carefully by knowing that it won't work in all cases. Can you share screenshot of the topologies you are trying to simulate ?

Best regards,
Pierre

Answer 17 · 2022-04-14T14:05:37.000Z

Hello @BonneelP ,

there is no specific topology that I want to simulate. But I think it would be great to have (non UD) Bore and Notch available.
Especially automated topology optimizations (using pyleecan optimization tools) would be a use case. Imported DXF Bore (+Notch) may be difficult in that case.

So far I think I have finished the major work on that feature. If you got some time it would be great if you could do some review, since there are some changes to current methods and functions. Also I removed yoke_notch and added 'is_yoke' property to notch.
Still I got to check that yoke notches are not broke by my changes, run unit test and add some other notch merge methods.

I hope you are okay with my changes and wait for your feedback.

Best regards, Sebastian

Edit:
PR #529
... I also added some TreeEdit features, i.e. now it is possible to append/remove list/dict elements. This is somehow useful if you want to test bore and notch from within the GUI :-)

Answer 18 · 2022-04-18T21:12:17.000Z

Hello Sebastian,

I think it would be great to have (non UD) Bore and Notch available.

I agree. Having a global rule that works whatever the Bore schematics could be complicated, but maybe we can start by having different strategy to merge notch on a BoreFlower.

Also I removed yoke_notch and added 'is_yoke' property to notch.

At first I was not convinced, by removing yoke_notch, since it makes it "harder" to detect if there are notches on the bore or the yoke (all notches object in the list must be checked). But a "has_notch(is_bore)" method could do the trick (and maybe get_notch_list(is_bore)).
On a second though, maybe we can go one step further: what about adding "is_bore=True" as a property of Slot (instead of notch) ? It should be "easy" to implement as we already use normalized method "get_Rbo" and "is_internal". It could be useful to create LamSlotMulti with both slot on bore and yoke (maybe needed if N lamination > 2 ?). It would also enables to remove the trick we use for yoke notches were we temporally change lamination.is_internal. What do you think ?

I will have a look on the PR this week.

Best regards,
Pierre

Answer 19 · 2022-04-19T16:13:27.000Z

Hello,

Regarding pyleecan/Functions/FEMM/get_sliding_band.py, if I understand well, you compute the position of the lamination point on the x axis to draw the segment on the sliding band side. At first I was first thinking about adding surfaces (as for the notches) to "close" the cylinder (is_circular_radius=True) and cutting them (as for the notches). But this solution works and is maybe easier than defining these surfaces. The only reason to define these surfaces would be to set different mesh properties in FEMM but I don't think it should be required as its in the airgap.

Best regards,
Pierre

Answer 20 · 2022-05-31T20:12:10.000Z

Hello Sebastian,

We are about to start the work to add the notches within the GUI (as a popup of the Lamination step). I will have some time to carry on the work on this PR, do you mind if I work directly on your branch (I should still have the access).
What do you think about my proposal to have "is_bore=True" as a property of slot ? I can do this modification as well.

Best regards,
Pierre

Answer 21 · 2022-06-01T05:46:00.000Z

Hello Pierre,

notches in the GUI will be great. You can also try to implement "is_bore=..." property.
I hope that I will also have some time to implement other merge algorythms the next days.

Best regrads, Sebastian

Answer 22 · 2022-06-09T19:50:34.000Z

Hello Sebastian,

I took the opportunity of this PR to completely reorganize the get_bore_desc and related methods. As we discussed in this issue, these methods were old and becoming unreadable as several unexpected methods/features were added afterwards.

The first principal of the new organization is to handle the bore and the yoke as the same thing (the "is_yoke" parameter was a great idea, thank you :) ). Now we have the method "build_radius_desc(is_bore)" and "build_radius_lines(is_bore)" and the slot have the "is_bore" property. For more readability "has_notch(is_bore)" and "has_slot(is_bore) were added. So now it is possible to have a Lamination with slot on both bore and yoke:

As I wanted to have bore and yoke identical, I also introduce "Lamination.yoke" (which is a "Bore" object ^^"):

Sym is available for both:

"desc_list" are now more consistent from method to method (with always labels and lines) and the slot/notch description list are merged before generating the radius lines. Now when there is a "shape" for bore/yoke and notch or slot a "Bore.merge_slot" method (that still needs to be written) is called. As this is a method of Bore, polymorphism is possible and properties can be added to select the merge method. It also remove this very particular case from the "normal" workflow which simplify the code. merge_slot take the notch/slot description as an input to generate the proper lines. Technically with this organization we can set bore shape on a lamination with winding/magnet slot.

All the tests (except one) are working, I plan to write some other tests to check that FEMM works with Bore object and sym, move your "merge_translate" code to a dedicated method and maybe provide a "merge intersect" method for Bore. I also have few correction to make but the main modifications are done and it should greatly simplify what we were talking about.

Best regards,
Pierre

Answer 23 · 2022-06-17T13:51:53.000Z

Hello all,

I'm finally done with this issue, the PR #529 and the issue #24 🥳
All the tests are working and I have added a validation case wih Bore and Notches (and even symmetry!):

I think that the code can still be improved since I had to remove the notch air surfaces and then the airgap mesh property is used inside the notches (and on "air" part of the Bore shape).

We may solve that by adding a line at Rbo+airgap/10 for instance.

As this kind of topology is complexe I'm not sure that the merge methods will work in all cases. I may come back to the code if some errors are found on existing machines.
The method translate can also be added later on by taking inspiration from the "intersect" method (normally is would "just" require to replace the "split_point" from the notch lines to a translate).

Thank you Sebastian for starting this work it was long overdue :)

Best regards,
Pierre

Answer 24 · 2022-06-22T08:31:05.000Z

Hello @BonneelP

thank you for this great work!

Although this issue is closed, ...
I finally starting to update my forks again and therefore review some of the changes.
It seems you discarded the 'move_translate' code for now, so I got to reimplement it?! (Don't worry about that.)

Do you have some example figures to show the actual merge methods. Especially I don't understand method 0.

Best regards,
Sebastian

Answer 25 · 2022-06-22T09:18:07.000Z

Hello,

Sorry for the move_translate, I wanted to add the code in merge_translate but I didn't managed to find it in the commit history to add it back :/ This merge method would be really helpful, I think that you can take inspiration from the merge_intersect to find where the bore and slot are colliding to apply the proper translation.

Regarding the examples, you can have a look at Tests/Validation/Magnetics/test_FEMM_periodicity.py:test_Bore_sym. Most of the figure above comes from this test and there are a stator with merge=0 and a rotor with merge=1.
Regarding merge 0, it "just" add segments between bore lines and slot/notch to "connect the dots". In the stator case, the bore lines are bellow the notches so there is no intersection between bore and slot lines, so the slot lines are "extended" by the joining segment.

I not completely sure that all methods will work in all cases, so if you want we can reopen this PR and carry on the work ;)

Best regards,
Pierre

Answer 26 · 2022-06-29T10:02:30.000Z

Hello @BonneelP

there is still some issue. If you have a look at the following figure, the sym != 1 plot has some missing notches. Maybe you can check if that also happen on your side.

Best regards, Sebastian

Answer 27 · 2022-06-29T10:13:43.000Z

... now this seems to be one of this cases that doesn't work.

Answer 28 · 2022-06-29T10:25:14.000Z

As I said, I expected that some configurations could create some issues. Can you share your machines ?

Answer 29 · 2022-06-29T11:15:14.000Z

Toyota_Prius_bore_notch.zip

Answer 30 · 2022-06-30T18:37:29.000Z

I think that I understood the issue:

In fact we call the split methods only by checking the opening angles. In this case the opening angle is not on the cutting line but the slot is larger than its opening angle and collide with the sym line. I will try to correct that by always cutting the first and last notch.
I guess that the issue was already there before the reorganization. It is only linked to the notches and bore description.
Thank you for spotting that, this is a case that can easily happen with parameter sweep :)

Best regards,
Pierre

Answer 31 · 2022-06-30T21:12:26.000Z

Hello sebastian,

I found a way to correct the plot method but regarding FEMM I don't think that we can handle that:

There are two main issues:

Regarding yoke side lines, we would need to detect that 3 lines are required and set all the proper boundary conditions (on both sides). We already have something for ventilation/cooling ducts but it would be tricky to extend it to this particular case
By cutting the line we create a small "triangle surface" that would need to be handled (to set properties)

Both issues confirm that it was not doable before ^^"

I would say that the best way to correct that would be to detect that case in the build_radius_desc method and remove the radius arc from 0 to first notch. The yoke side line would then ends on the intersection (as it would on the "normal" sym case) and in get_notches_surf the triangle surface could be returned (the BC on the yoke side needs to be handled as well). But even then we would have the yoke side line issue on the other side (can be solved by editing build_yoke_side_line).

And I expect that with a negative alpha the situation can be reversed (the triangle would be on the left side of the yoke).

I won't have time to work on this particular case in the near future, I'm focusing on #543 and updating all the tutorial then the website. I will come back later to this issue. Should we rename it or create a new one ?

Thanks again for finding this very particular case,
Best regards,
Pierre