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

api: Add op.cinterface for C-level interoperability #1843

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Conversation

FabioLuporini
Copy link
Contributor

No description provided.

@FabioLuporini FabioLuporini added the API api (symbolics, types, ...) label Feb 21, 2022
@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #1843 (259c5f1) into master (5ebaf62) will increase coverage by 0.02%.
The diff coverage is 95.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1843      +/-   ##
==========================================
+ Coverage   89.51%   89.54%   +0.02%     
==========================================
  Files         209      210       +1     
  Lines       34875    34939      +64     
  Branches     5261     5271      +10     
==========================================
+ Hits        31218    31285      +67     
+ Misses       3164     3161       -3     
  Partials      493      493              
Impacted Files Coverage Δ
devito/operator/operator.py 90.54% <86.66%> (+0.05%) ⬆️
devito/ir/iet/visitors.py 82.64% <95.45%> (+0.66%) ⬆️
tests/test_cinterface.py 100.00% <100.00%> (ø)
devito/finite_differences/elementary.py 94.61% <0.00%> (+0.29%) ⬆️
tests/test_dle.py 98.12% <0.00%> (+0.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ebaf62...259c5f1. Read the comment docs.

@@ -603,6 +603,39 @@ def cfunction(self):

return self._cfunction

def cinterface(self, force=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding name=None to the API (and a if not name before line 621) so that the caller can change the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the user can control that already via Operator(..., name=X) -- so here (unlike "classic" devito use model) we don't use the .soname but rather the actual Operator name.


def _operator_includes(self, o):
includes = super()._operator_includes(o)
includes.append(c.Include("%s.h" % o.name, system=False))
Copy link
Contributor

Choose a reason for hiding this comment

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

why this not in _operator_includes ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well when we normally execute devito and run op.apply there's no Operator-specific header file to include --it's all in the .c, no? :)

@@ -467,9 +468,34 @@ def visit_HaloSpot(self, o):
body = flatten(self._visit(i) for i in o.children)
return c.Collection(body)

def visit_Operator(self, o):
blankline = c.Line("")
# Operator-handle machinery
Copy link
Contributor

Choose a reason for hiding this comment

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

handling?

xfilter = lambda i: not isinstance(i, public_types)

typedecls = [i._C_typedecl for i in o.parameters
if xfilter(i) and i._C_typedecl is not None]
Copy link
Contributor

Choose a reason for hiding this comment

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

is not None can be safely dropped I think?

xfilter = lambda i: not isinstance(i, public_types)

typedecls = [i._C_typedecl for i in o.parameters
if xfilter(i) and i._C_typedecl is not None]
Copy link
Contributor

Choose a reason for hiding this comment

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

is not None can be safely dropped I think?

if not i.local:
continue
typedecls.extend([j._C_typedecl for j in i.root.parameters
if xfilter(j) and j._C_typedecl is not None])
Copy link
Contributor

Choose a reason for hiding this comment

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

is not None can be safely dropped I think?

if not i.local:
continue
typedecls.extend([j._C_typedecl for j in i.root.parameters
if xfilter(j) and j._C_typedecl is not None])
Copy link
Contributor

Choose a reason for hiding this comment

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

is not None can be safely dropped I think?

@mloubout
Copy link
Contributor

CI error is a stupid codecov failing to upload this is GTG

@mloubout mloubout merged commit 0b00df4 into master Mar 1, 2022
@mloubout mloubout deleted the cinterface branch March 8, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API api (symbolics, types, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants