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

Handle 1D arrays with Field API + add options to fieldType.py generator #19

Merged
merged 3 commits into from
Jan 19, 2024

Conversation

pmarguinaud
Copy link
Collaborator

The purpose of this branch is to add Field API classes for 1D arrays. These classes are similar to exising classes, except that :

  • 1D Field classes do not have the GET_VIEW method
  • no 1D array class exist
  • 1D Field classes have no corresponding gang class
  • 1D Field classes cannot be gathered/scattered with the existing gather/scatter object

Some extra attributes have been added to the fieldType object:

  • ganged : can this class be part of a gang
  • hasView : does this class have the GET_VIEW method

The fieldType generator has been added three extra arguments:

  • ganged (True/False), if set keep only classes with matching attribute
  • hasView (True/False), if set keep only classes with matching attribute
  • alias (True/False), if set keep only classes with matching attribute

@pmarguinaud pmarguinaud requested a review from awnawab January 16, 2024 15:45
Copy link
Collaborator

@awnawab awnawab left a comment

Choose a reason for hiding this comment

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

Thanks @pmarguinaud for these additions! It all looks good to me 👍 The only point I raised is an optional change to getFieldTypeList, which I think makes it a little simpler and more readable.

fieldType.py Outdated

def getFieldTypeList (ranks=[2,3,4,5], kinds=kinds):
return [fieldType (kind=kind, rank=rank) for (kind) in kinds for rank in ranks]
def eqv (a, b):
Copy link
Collaborator

Choose a reason for hiding this comment

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

[optional style change]

Perhaps this could be simplified a little:

def getFieldTypeList (ranks=[1,2,3,4,5], kinds=kinds, hasView=False, alias=True, ganged=False):

  l = [fieldType (kind=kind, rank=rank) for (kind) in kinds for rank in ranks]

  if hasView:
    l = [ft for ft in l if ft.hasView]

  if not alias:
    l = [ft for ft in l if not ft.alias]

  if ganged:
    l = [ft for ft in l if ft.ganged]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hello Ahmad,

I need to make selection with True/False values, so I have to keep the eqv function. I agree that the [ft for ft in l if ...] looks better then the filter function, so I will use it. But I will keep the eqv.

Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't that just

l = [ft for ft in l if hasView is not None and hasView == ft.hasView]

or

if hasView is not None:
    l = [ft for ft in l if hasView == ft.hasView]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you Lukas, it works !

In Fortran, it is not possible to compare logicals with ==, but with python it works.

Copy link
Collaborator

@mlange05 mlange05 left a comment

Choose a reason for hiding this comment

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

I agree with the Python style suggestions, but otherwise looks good to me.

@pmarguinaud pmarguinaud merged commit 27c44be into ecmwf-ifs:main Jan 19, 2024
1 check passed
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.

4 participants