Skip to content

Conversation

@krober10nd
Copy link
Collaborator

@krober10nd krober10nd commented Jan 31, 2021

  • this is a breaking change that will need some time and extensive testing to land
  • addresses more intuitive deactivation of msh.clean() options #180
  • msh.clean()'s passive mode does not apply nodal valency reduction now.
  • TODO: better handling of nodal attribute migration when applying moderate and aggressive cleaning.

Keith Roberts and others added 4 commits January 31, 2021 18:09
* Does not apply nodal valency reduction now. 
* Todo: better handling of nodal attribute migration when applying moderate and aggressive cleaning.
@krober10nd
Copy link
Collaborator Author

If new nodes are added (such as is the case when running the nodal valency reducer) then this section is problematic for mapping nodal attributes (the intersection) as this will only map nodes that existed in the old mesh onto the new mesh. Thus, new nodes added in the cleaning operations will get default values.

            % f13
            if ~isempty(obj.f13)
                obj.f13.NumOfNodes = length(ind);
                for att = 1:obj.f13.nAttr
                    % Get the old index for this attribute
                    idx_old = m_old.f13.userval.Atr(att).Val(1,:);
                    val_old = m_old.f13.userval.Atr(att).Val(2:end,:);
                    % Only keep idx and val that is common to ind and map to ind
                    [~,ind_new,idx_new] = intersect(idx_old,ind);
                    val_new = val_old(:,ind_new);
                    % Put the uservalues back into f13 struct
                    obj.f13.userval.Atr(att).Val = [idx_new'; val_new];
                    obj.f13.userval.Atr(att).usernumnodes = length(idx_new);
                end
            end

@krober10nd
Copy link
Collaborator Author

Okay, this is now working on my end with running bound_con_init. In general, map_mesh_properties can now support the case of deletion and addition of new nodes. For the latter case, it uses nearest neighbor interpolation to put values in for those nodal attributes.

In summary, all levels of mesh cleaning can be used with the migration of the nodal attributes.

Copy link
Collaborator

@WPringle WPringle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great, will definitely come in handy. I think we can also use the new mapper in the BoundCr routine.

@krober10nd
Copy link
Collaborator Author

krober10nd commented Feb 4, 2021

I think we can also use the new mapper in the BoundCr routine.

I'm on the fence about that because some nodal attributes depend on the bathymetry (e.g., primitive weights) and the bathy. slope should be recalculated if the nodes change. Also, the migration of boundaries is non-trivial and often has to be re-calculated anyway.

I'd rather the more advanced user do it afterwards from the old mesh to the new updated mesh via

m_old = map_mesh_properties(m_new,'msh_old',m_new); 

I think the application of nearest node mapping is suitable for the clean method however because this operation is often performed to clean up mesh quality or make boundaries traversable after the model is incrementally modified for instance. In general, for small changes.

@WPringle
Copy link
Collaborator

WPringle commented Feb 4, 2021

Fair enough. Yeah I think an obvious problem with using the map_mesh_properties as defined now after you did the BoundCr is that the bathymetry is already updated and there is no option to disable that mapping over at present. Let's just merge this in as you have it now.

@krober10nd krober10nd merged commit d6fbc5f into Projection Feb 4, 2021
@krober10nd krober10nd deleted the improve_clean_interface branch February 4, 2021 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants