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

Draft: add basis transformation procedure #984

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

AdelekeBankole
Copy link
Collaborator

No description provided.

Comment on lines 1264 to 1268
if (!q_weight_1d)
points = Q;
else
points = Q;
weights = q_weight_1d;
Copy link
Member

Choose a reason for hiding this comment

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

Note that the indentation here does not show the scope. Your editor should be able to re-indent to make the actual scope of the if and else clear.


@ref Utility
**/
int CeedHaleTrefethenStripMap(CeedInt Q, CeedInt rho, CeedScalar *g, CeedScalar *g_prime) {
Copy link
Member

Choose a reason for hiding this comment

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

If this is a public function, it'll need to be declared in a public header.

@@ -1270,6 +1322,8 @@ int CeedGaussQuadrature(CeedInt Q, CeedScalar *q_ref_1d,
q_ref_1d[i] = -xi;
q_ref_1d[Q-1-i]= xi;
}
// Call transformed Gauss-Legendre quadrature
Copy link
Member

Choose a reason for hiding this comment

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

You probably can't just call it here because you can't change the behavior of the existing interfaces, but you'll add a new interface that creates a Gauss quadrature and then transforms it.

Copy link
Member

@jedbrown jedbrown left a comment

Choose a reason for hiding this comment

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

You have reference code in Julia so the job here is mainly to be able to create the same results. So start with a particular example in Julia, like the points and weights for 5 Gauss quadrature points using the $\rho=1.4$ transform.

@@ -1231,28 +1231,30 @@ int CeedBasisDestroy(CeedBasis *basis) {

@ref Utility
**/
int CeedHaleTrefethenStripMap(CeedInt Q, CeedInt rho, CeedScalar *g, CeedScalar *g_prime) {
CeedScalar tau, d, C, PI2, PI = 4.0*atan(1.0);
int CeedHaleTrefethenStripMap(CeedInt Q, CeedScalar rho, CeedScalar *g, CeedScalar *g_prime) {
Copy link
Member

Choose a reason for hiding this comment

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

This still isn't a function $g(s)$ as we've discussed. You'll need to take const CeedScalar *s as an argument, then write g[i] as a function of s[i].
image

}
return CEED_ERROR_SUCCESS;
}

/**
@brief Transform quadrature by applying a smooth mapping = (g, g_prime)

@param Q Number of quadarture points
@param Q Number of quadrature points
Copy link
Member

Choose a reason for hiding this comment

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

In general, every argument has to be documented. I don't know what you intend this function to do; maybe it can be deleted or maybe it needs to take a CeedBasis as input and create a CeedBasis as output.

CeedScalar rho;
CeedScalar *g, *g_prime;

CeedHaleTrefethenStripMap();
Copy link
Member

Choose a reason for hiding this comment

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

We talked about checking by spot-checking accuracy relative to figure 7.1a. How would you test that? Take 20-point Gauss quadrature and integrate the function, then transform the quadrature points and weights, and integrate that way in $[-1, 1]$. The second should be more accurate.

image

@jeremylt jeremylt added the 0-WIP label Jun 27, 2022
@jeremylt jeremylt marked this pull request as draft September 6, 2022 23:18
@jeremylt
Copy link
Member

What's the plan for this PR?

@jedbrown
Copy link
Member

I think that's going to be a choice for @AdelekeBankole and perhaps another student will take interest in driving this further. I think it causes no harm to stay open for a while.

@AdelekeBankole
Copy link
Collaborator Author

Yes that's correct, I plan to move move this PR forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants