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

Add function space class. #477

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ketch
Copy link
Member

@ketch ketch commented Nov 17, 2014

The functionSpace class sits between State and Patch.
A function space lives on a patch but also has a number of DOFs.
It is intended to match the PETSc DMDA abstraction.
This will allow us to reduce the number of DA's and Vecs we use,
by reusing DA's from the main State object in the _register objects
in SharpClaw.

It is also anticipated that this abstraction will be useful when
implementing DG finite element methods.

@jedbrown please take a look and let me know if this is what you had in mind. I think there are still some improvements to be made in terms of not creating so many local vectors.

The functionSpace class sits between State and Patch.
A function space lives on a patch but also has a number of DOFs.
It is intended to match the PETSc DMDA abstraction.
This will allow us to reduce the number of DA's and Vecs we use,
by reusing DA's from the main State object in the _register objects
in SharpClaw.

It is also anticipated that this abstraction will be useful when
implementing DG finite element methods.
@@ -1,5 +1,53 @@
import clawpack.pyclaw

class functionSpace(clawpack.pyclaw.state.functionSpace):
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to not capitalize the first letter of the class names?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but now I realize that's inconsistent with the rest of PyClaw. I'll change it.

@mandli
Copy link
Member

mandli commented Nov 17, 2014

I am a bit confused now as to how this abstraction plays with the State object. I figured that the FunctionSpace object would always be present in a State object rather than only being present in the PETSc version of the State. It's more overhead I guess but it seems cleaner to me.

@ketch
Copy link
Member Author

ketch commented Nov 18, 2014

I figured that the FunctionSpace object would always be present in a State object rather than only being present in the PETSc version of the State.

That's right. Did you see https://github.com/clawpack/pyclaw/pull/477/files#diff-f5c0f62513f67848fc9cc6eb2b3f8446R10 ?

@ketch
Copy link
Member Author

ketch commented Nov 18, 2014

I'm doing more work on this, so don't bother spending much time on reviewing it until I've pushed more.

@mandli
Copy link
Member

mandli commented Nov 18, 2014

Ah, I see, I was confused by the variable naming.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.24%) when pulling 93a8fdf on ketch:function_space into 011422a on clawpack:master.

@ketch
Copy link
Member Author

ketch commented Nov 18, 2014

Okay, it's much improved now and the tests are passing. I still feel like all this code is not as elegant as it could be. But I can't put my finger on how to improve it at the moment.

@ketch ketch changed the title [WIP] Add function space class. Add function space class. Dec 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants