Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bezier Field Solution Transfer #321

Draft
wants to merge 62 commits into
base: develop
Choose a base branch
from

Conversation

avinmoharana
Copy link
Contributor

This pull request adds the transfer of bezier solution fields.

Nightly Bot and others added 30 commits August 29, 2019 00:13
Note that blending is used internally in Bezier Shapes. More
specifically when a BezierCurver object is set with a non-zero third
argument, Bezier shapes for tets and triangles might behave differently.
Therefore, we needed to make sure the blending orders are set to zero at
the beginning of the convertInterpolationgFieldsToBezier routine and
then reverted back to the original values at the end.

There still seems to be necessary to construct a new BezierCurver object
with last argument 0 for the bezier fields to work properly.
…ier_fields

Conflicts:
	crv/crvBezierFields.cc
@cwsmith
Copy link
Contributor

cwsmith commented Aug 24, 2020

Would you please rebase this branch on develop?

@@ -20,6 +22,7 @@ set(SOURCES
crvTables.cc
crvQuality.cc
crvVtk.cc
crvBezierFields.cc
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

crvBezierFields.cc seems to have been added twice!

@@ -37,6 +41,13 @@ int getBlendingOrder(const int type);
/** \brief count invalid elements of the mesh */
int countNumberInvalidElements(apf::Mesh2* m);

/** \ brief converts field f to Bezier entity wise */
void convertInterpolationFieldPoints(apf::MeshEntity* e,
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana
Is this used by outside users or only internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
The converInterpolationFieldPoints is not currently used outside, but it would be nice to have an entity-specific convert routine.

Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

What do you mean by entity-specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
I meant while testing an operator we construct a toy example where this routine can get the control points/ interpolation points for a specific edge or a face.
The routine convertInterpolationFieldPoints is defined in crvBezierFields.cc and is used in crvBezierSolutionTransfer.cc and we don't have a header file defined for crvBezierFields.

apf::Field* f, int n, int ne, apf::NewArray<double> &c);

/** \brief converts field f, which is interpolating to Bezier */
void convertInterpolatingFieldToBezier(apf::Mesh2* m_mesh, apf::Field* f);
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana
Is this used by outside users or only internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
This can be used outside as well. If we have an interpolating field defined outside then we can convert to Bezier from outside.

Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana if it is not used outside, it should not be here. If they are design issues that prevent this we should discuss and resolve those appropriately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
We want this to be accessible from outside. Can we not keep the current design?

@@ -203,6 +214,10 @@ void writeInterpolationPointVtuFiles(apf::Mesh* m, const char* prefix);
int getTriNodeIndex(int P, int i, int j);
int getTetNodeIndex(int P, int i, int j, int k);

/** \brief adds bezier solution transfers */
ma::SolutionTransfer* setBezierSolutionTransfers(
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

Is this used by outside users or only internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
No this not used outside.

Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

then it should be made static. It also seems that this file is not a correct location for this. In that case, it has to be moved to the file it is defined. Where is the implementation (the actual code)? Can you just remove this declaration make the implementation static in the file it is defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
The setBezierSolutionTransfers() is defined in crvBezierSolutionTransfer.cc and is called inside the adapt routine of crvAdapt.cc.
The current implementation uses the createBezierSolutionTransfer() (defined static in crvBezierSolutionTransfer.cc) to access the bezier fields and the setBezierSolutionTransfers adds those to solutionTransfer.

I can make the createBezierSolutionTransfer() accessible and define the setBezierSolutionTransfers routine in crvAdapt.cc, or define both only in crvAdapt.

crv/crvAdapt.cc Outdated
@@ -194,6 +195,16 @@ static void flagCleaner(crv::Adapt* a)
}
}

void getAllBezierFields(ma::Mesh* m, std::vector<apf::Field*>& fields)
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

if this is used only in this file, it should be made static.

@@ -56,6 +56,8 @@ set(HEADERS
ma.h
maInput.h
maMesh.h
maMap.h
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

Are these two need to be here, or is this done for debugging? If this is done for debugging, please revert this back.

ma/maRefine.cc Outdated Show resolved Hide resolved
ma/maRefine.cc Show resolved Hide resolved
ma/maShape.cc Outdated Show resolved Hide resolved
ma/maSize.cc Outdated Show resolved Hide resolved
@@ -37,6 +41,13 @@ int getBlendingOrder(const int type);
/** \brief count invalid elements of the mesh */
int countNumberInvalidElements(apf::Mesh2* m);

/** \ brief converts field f to Bezier entity wise */
void convertInterpolationFieldPoints(apf::MeshEntity* e,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
The converInterpolationFieldPoints is not currently used outside, but it would be nice to have an entity-specific convert routine.

apf::Field* f, int n, int ne, apf::NewArray<double> &c);

/** \brief converts field f, which is interpolating to Bezier */
void convertInterpolatingFieldToBezier(apf::Mesh2* m_mesh, apf::Field* f);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
This can be used outside as well. If we have an interpolating field defined outside then we can convert to Bezier from outside.

@@ -203,6 +214,10 @@ void writeInterpolationPointVtuFiles(apf::Mesh* m, const char* prefix);
int getTriNodeIndex(int P, int i, int j);
int getTetNodeIndex(int P, int i, int j, int k);

/** \brief adds bezier solution transfers */
ma::SolutionTransfer* setBezierSolutionTransfers(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
No this not used outside.


void convertInterpolatingFieldToBezier(apf::Mesh2* m_mesh, apf::Field* f)
{
// TODO: to be completed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
There is no assumption that the blending order of the entities is 0, i.e. the convert to Bezier Field should work with a non-zero blending order, as is implemented for the coordinate field. Should I remove the TODO from here?

crv/crvShapeHandler.cc Show resolved Hide resolved
apf::NewArray<double> value;
};
return 0;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah

we added this routine to print the field name that undergoes solution transfer onto the output console.

Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

things that are added for debugging should be reverted back to their original state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
Removed getTransferFieldName().

int minDim;
};
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah

From line 216 to 245, it was a fix to account for piecewise constant energy field, where we conserve the total energy of the cavity on mesh adapt. This was a specific fix to the problem and should not be generalized. Should I remove this part?

EntityArray& oldElements,
EntityArray& newEntities);
private:
int minDim;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
We modified the structure of the code to make the routines usable outside maSolutionTransfer.

crv/crvAdapt.cc Show resolved Hide resolved
val[k] = 0.;
apf::setComponents(f, newEntities[i], j, &(val[0]));
}
repositionInteriorFieldWithBlended(mesh,f,newEntities[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana
why are we getting involved with blending here?

getBezierTransformationMatrix(parentType,childType,P,A,vp);
mth::multiply(Ai[apf::Mesh::typeDimension[childType]],A,B);
for (int j = 0; j < ni; ++j){
apf::Vector3 point(0,0,0);
for (int k = 0; k < np; ++k)
for (int k = 0; k < np; ++k) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

can you remove the extra bracket for this for loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
Removed the extra bracket.

@@ -100,7 +164,7 @@ class SolutionTransfers : public SolutionTransfer
virtual void onCavity(
EntityArray& oldElements,
EntityArray& newEntities);
private:
//private
Copy link
Contributor

Choose a reason for hiding this comment

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

@avinmoharana

Why is the private commented here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortezah
We accessed the field names for solution transfer from transfers which was earlier defined private. Reverted it back to private.

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