bugpyFormex - Bugs: bug #40662, selectProp and related have no...

 
 

bug #40662: selectProp and related have no option for compacting

Submitted by:  francesco <francio>
Submitted on:  Thu 21 Nov 2013 05:11:40 PM UTC  
 
Category: CoreSeverity: 3 - Normal
Item Group: Feature requestStatus: Fixed
Privacy: PublicAssigned to: Benedict Verhegghe <bverheg>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 22 May 2014 07:32:54 PM UTC, comment #10:

In 8576bb9 the default for (c)selectProp has also been changed to False. This closes the bug.

Benedict Verhegghe <bverheg>
Project AdministratorIn charge of this item.
Thu 22 May 2014 03:47:02 PM UTC, comment #9:

In 44dec27, compact=False has been made the default for select and cselect. We probably should do this also for selectProp and cselectProp.

Benedict Verhegghe <bverheg>
Project AdministratorIn charge of this item.
Tue 13 May 2014 06:38:25 AM UTC, comment #8:

Hi
This bug can be closed. Though the suggestion of benedict have been implemented
select/cselect still use compact=True

Benedict can you close this bug and implement the changes if any?

francesco <francio>
Project Member
Fri 29 Nov 2013 01:19:32 PM UTC, comment #7:

Since I have not seen a deciding number of responses yet, here is a proposal of what I think to do:

- select/cselect use compact=False by default: they are intended to select some components of a Mesh but the user wants to keep working within the framework of that Mesh.

- clip/cclip become wrapper functions that call select with the option compact=True: they are intended for the user who wants the result of the selection for the continuation of his work.

Other element selection mechanisms should by preference just return a list of element numbers: these can then be put in any of the methods above to yield the corresponding Mesh. All the other methods then do not need a compact= option, nor do they need a complementary operation. And there is the added benefit that you can just get the element number list if that is all you need.

The new adjacentTo method works that way. I think we should change connectedTo in the same sense, and remove notConnectedTo.

Benedict Verhegghe <bverheg>
Project AdministratorIn charge of this item.
Mon 25 Nov 2013 09:22:11 AM UTC, comment #6:

Well, I see two reasons to keep the compact option, even if we make the default False.

- A lot of people now have probably code with 'compact=False' in it. That code would break if we just remove the option.

- In GUI functions, it is now easy to add the option to the input dialog as an _I('compact',False) item and execute the function(**res) with the dialog results. If the option is removed, we have to add everywhere a piece of code

It should even be more complex: the 'compact' should be removed from the res first, and the value remembered.

Benedict Verhegghe <bverheg>
Project AdministratorIn charge of this item.
Mon 25 Nov 2013 09:05:53 AM UTC, comment #5:

Hi everybody,
I would like to propose the following:

-IMMEDIATELY REMOVE the option compact from (c)select and (c)selectProp, getBorderMesh, getFreeEntitiesMesh,getFreeEdgesMesh,connectedTo. You can perfectly do it afterwards with a .compact() or a .fuse().compact() or better as a .fuse().compact().renumber(), without adding mem or cpu time. Also, other codes such as Abaqus and VTK do not alter the coords when working on connectivity/selection. If you want to compact (and/or renumber) you need to use a 'clean' in VTK and 'removeUnneededNodes' in Abaqus CAE. In a .inp file or .vtk you normally have unused nodes if you do not remove them first. The beginning user will probably not care if his model has unused nodes, but he would probably like that a node nr 3 will always be node nr 3 also after a .getBorderMesh or a .select.

-in functions where the .compact() is not executed at the end, I would keep the compact option. Most of the times it would reduce the memory usage (especially when there are for loops as in splitRandom).

-clipping TriSurface and GUI functions can use a compact() when needed (e.g. after clipping). As already suggested, clip and cclip are redundant (already for a long time actually) and can be replaced by (c)select.

gianluca de santis <gianlucadesi>
Project Member
Mon 25 Nov 2013 08:02:14 AM UTC, comment #4:

I agree that a default compact=False would give surprising results to the beginning user, especially when using the GUI menu functions (e.g. clipping a surface).

Sofie Van Cauter <svcauter>
Project Member
Sun 24 Nov 2013 04:49:47 PM UTC, comment #3:

On one hand I think it is better to compact by default. The beginning user would be surprised that a Mesh contains unneeded nodes. We would have to explain the implementation in more detail before he can start using Mesh. Experienced users like yourself will know why, when and where they have to add the compact=False option.

On the other hand, compact=False may be used more than compact=True, so it would make sense to make False the default.
I think however that the default should be the same, not only for selectProp and select, but probably for all the methods having the compact argument. There are currently 14 of them: 13 with True as default, only 1 with False as default.

An asset of setting default=False would be that we can remove the option altogether: it is not more difficult to write M.select(something).compact() than M.select(something,compact=True).

The change to default compact=False (or removal of the compact option) would of course require us to check carefully the usage of the said 14 methods. Some will be no problem: selectProp and cselectProp could be directly set to False, since the withProp also worked that way (is that correct?). clip/cclip can probably be deprecated anyway: they seem to do the same as select/cselect. TriSurface.removeDegenerate can loose the option: Mesh.removeDegenerate has it neither.

So, my idea is the following: if enough developers/users (let's say 5) ask for a default compact=False, I will start changing (and later removing the option).

Otherwise, I will make my one decision, someday.

Benedict Verhegghe <bverheg>
Project AdministratorIn charge of this item.
Sun 24 Nov 2013 02:07:02 PM UTC, comment #2:

It works with the new changes and after commit e978091.
Though I ll suggets to set the deafult compact to False.
Normally while working with properties I think the user will rather keep the original positions than the compacted mesh.
I left the discussion open only for this matter, else can be closed

francesco <francio>
Project Member
Thu 21 Nov 2013 06:18:01 PM UTC, comment #1:

In commit 4f238bc a 'compact' argument has been added to selectProp and cselectProp.

Geometry.select has compact=True as default because that is what a beginning user will probably expect. An experienced user will know what this argument is about, and he can set it to False.

If the default True is annoying, we can discuss if it should be changed, or if we need to provide a configuration for the default value.

Notice that select/cselect have been moved from Formex/Mesh up to Geometry.

Please test the correct working of (c)select(Prop) and report.

Benedict Verhegghe <bverheg>
Project AdministratorIn charge of this item.
Thu 21 Nov 2013 05:11:40 PM UTC, original submission:

why Geometry.select have default compact=True?
moreover selectProp and related have no option for compacting.

If there is no good reason i will restore all defaults to compact=False as it was before with deprecated withProp and related selection tools

francesco <francio>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by gianlucadesi (Posted a comment)
  • -unavailable- added by svcauter (Posted a comment)
  • -unavailable- added by bverheg (Posted a comment)
  • -unavailable- added by francio (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 22 May 2014 07:32:54 PM UTCbverhegOpen/ClosedOpen=>Closed
    Sun 24 Nov 2013 02:07:02 PM UTCfrancioStatusReady For Test=>Fixed
    Thu 21 Nov 2013 06:18:01 PM UTCbverhegStatusNone=>Ready For Test
      Assigned toNone=>bverheg

    Back to the top


    Powered by Savane 3.1-cleanup1