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

Modernising PyOP2 #615

Closed
connorjward opened this issue May 7, 2021 · 5 comments
Closed

Modernising PyOP2 #615

connorjward opened this issue May 7, 2021 · 5 comments

Comments

@connorjward
Copy link
Collaborator

Since I'm going to be spending a lot of time digging around PyOP2 over my PhD I think it would be good to get a list of old features that can be removed/refactored so I can tackle them along the way. I was thinking of creating a GitHub Project to track them all.

To start with I'd like to know what people think about the following:

  1. Split apart base.py into arg.py, dat.py, set.py etc (mentioned here).
  2. Merge petsc_base.py and sequential.py into these files. My understanding is these were created when it was thought that we might want to support multiple backends but this is no longer the case.
  3. Expunge COFFEE. I'm not very knowledgeable about the project but I know that we now prefer to use Loopy. Is there any value in keeping it?
  4. Remove caching.py (see below).
  5. Redesign the versioning. I understand versioneer very little but Lawrence mentioned that he would like to expunge the forking that goes on in _version.py so I thought that this should be a part of the discussion.

For the caching mentioned above, one of the things that I am working on at the moment is trying to treat parloops as generated code, much like a TSFC kernel, so they don't hold any data references. This means that we can focus on caching parloops outside of PyOP2 so we could remove much of the caching that we currently do internally.

Please do share any other ideas that you think of.

@wence-
Copy link
Member

wence- commented May 7, 2021

Expunge COFFEE. I'm not very knowledgeable about the project but I know that we now prefer to use Loopy. Is there any value in keeping it?

There are a bunch of places in firedrake where we still manipulate coffee rather than loopy kernels to paste them together. TBH this is mostly to paste strings together so one could build loopy and turn it into a string. I think we never did it is all.

@wence-
Copy link
Member

wence- commented May 7, 2021

For the caching mentioned above, one of the things that I am working on at the moment is trying to treat parloops as generated code, much like a TSFC kernel, so they don't hold any data references. This means that we can focus on caching parloops outside of PyOP2 so we could remove much of the caching that we currently do internally.

Just FYI, there are two types of caching in PyOP2:

  1. Generated code caches. These are on-disk (and in-memory) caches that save the result of jitting, disentangling these from data is a good idea
  2. object caches that make many classes into singleton constructors. For example if you run s = op2.Set(1); d1 = op2.DataSet(s, 1); d2 = op2.DataSet(s, 1) we have that d1 is d2. This was an early design decision that we should now remove since we're no longer doing things this way (Firedrake manages these objects).

I think 2 was also done because we used object identity lookup in various other caches (but again that can go away). This was a kind of workaround for initially thinking that some objects would be throw-away create/destroy, but then realising that they needed to carry heavy data.

I think a redesign should do basically no caching and require the consumer of the API to hold on to objects that are "heavy". We should then strive to make throwaway objects as lightweight to construct as possible.

@connorjward
Copy link
Collaborator Author

Just FYI, there are two types of caching in PyOP2:

Ah yes thank you. We should definitely keep the former but I didn't make that clear.

Also, given that you haven't commented on the remaining items I brought up, am I safe to assume that you agree with them?

@wence-
Copy link
Member

wence- commented May 10, 2021

I think so

@connorjward
Copy link
Collaborator Author

This is quite an out-of-date discussion and my pyop3 work supercedes this. Closing.

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

No branches or pull requests

2 participants