Hi Andriy, Sorry for the delay. I hadn't noticed your previous reply. Your commits have been rebased/squashed/merged. All of your changes should be in the current master. If you confirm this, you can delete the touched_region_for_projected_fem branch yourself.
Thanks for the useful new function. Best regards Kostas On Tue, Mar 9, 2021 at 9:37 AM Andriy Andreykiv <andriy.andrey...@gmail.com> wrote: > Kostas, hi, > > Did you have a chance to review my latest commits? > Thanks, > Andriy > > On Fri, 5 Mar 2021 at 09:27, Konstantinos Poulios <logar...@googlemail.com> > wrote: > >> Thanks. I see your point about "intersected". I think >> "projected_target_region" is a good name. >> >> If you do this change I can squash and merge your commits. >> >> Best regards >> Kostas >> >> On Thu, Mar 4, 2021 at 2:06 PM Andriy Andreykiv < >> andriy.andrey...@gmail.com> wrote: >> >>> Hi Konstantinos, >>> >>> Replaced almost all the auto's. Regarding intersected_target_region, >>> that could be, but for me the word intersected would be appropriate if it >>> was interpolated_fem where the regions intersect. >>> Physically it feels like touch, but I agree that we have touch() in >>> context_dependencies which might be confusing. >>> Here I feel a need for the concept of projection. I personally need a >>> name that condenses the sentence "region that contains part of the target >>> where the source is projected on". Can we say >>> projected_target_region? We can also go with supported_target_region()? >>> >>> Let me know what you think, >>> Andriy >>> >>> >>> >>> On Thu, 4 Mar 2021 at 12:38, Konstantinos Poulios < >>> logar...@googlemail.com> wrote: >>> >>>> should we also briefly discuss the name of the function? In >>>> mathematical terms I would call it >>>> >>>> support_region >>>> >>>> But a more popularized name might be easier to understand in general. >>>> However, I do not like "touched" because "touch" is typically used in >>>> programming for denoting change in state, so your current name I understand >>>> it as a region that has state A and changes to state B. We could instead >>>> call it >>>> >>>> intersected_target_region >>>> >>>> or something similar. Other ideas? >>>> >>>> In general I think we should spend some effort in good names because >>>> once a name is in the API it is more difficult to remove. >>>> >>>> Best regards >>>> Kostas >>>> >>>> On Thu, Mar 4, 2021 at 12:31 PM Konstantinos Poulios < >>>> logar...@googlemail.com> wrote: >>>> >>>>> Hi Andriy, >>>>> >>>>> Thanks, I see how it can be useful. Could I ask you to reduce the use >>>>> of auto for this commit? For example it does not make much sense to use >>>>> auto for bool. In general my preference for the GetFEM codebase is to use >>>>> auto only if some type is particularly long and makes the code >>>>> significantly less readable. Otherwise the type of the variables is useful >>>>> information for people that will read and try to understand the code >>>>> later. >>>>> >>>>> There is also a typo in a comment. It should be "Gauss". >>>>> >>>>> Best regards >>>>> Kostas >>>>> >>>>> On Thu, Mar 4, 2021 at 11:32 AM Andriy Andreykiv < >>>>> andriy.andrey...@gmail.com> wrote: >>>>> >>>>>> Dear Yves and Konstantinus, >>>>>> >>>>>> Kind request to review and merge touched_region_for_projected_fem >>>>>> branch. >>>>>> It introduces a method for projected_fem that extracts a region from >>>>>> the target that is actually touched by the source. >>>>>> I use this region to integrate my mortar terms on. >>>>>> >>>>>> Best regards, >>>>>> Andriy >>>>>> >>>>>> >>>>>>
