Thank you, Kostas On Wed, 10 Mar 2021, 17:08 Konstantinos Poulios, <logar...@googlemail.com> wrote:
> 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 >>>>>>> >>>>>>> >>>>>>>
