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: Cleanup and improve SubFunction #2198

Merged
merged 5 commits into from
Sep 8, 2023
Merged

api: Cleanup and improve SubFunction #2198

merged 5 commits into from
Sep 8, 2023

Conversation

mloubout
Copy link
Contributor

@mloubout mloubout commented Sep 7, 2023

Fixes #2192

  • Fixes subs/rebuild of SparseFunction to properly propagate through the subfunctions (coords, ...)
  • Add tests

@mloubout mloubout added the API api (symbolics, types, ...) label Sep 7, 2023
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #2198 (68c29d4) into master (a7c3446) will increase coverage by 0.00%.
The diff coverage is 96.93%.

@@           Coverage Diff           @@
##           master    #2198   +/-   ##
=======================================
  Coverage   87.08%   87.08%           
=======================================
  Files         228      228           
  Lines       40579    40611   +32     
  Branches     7419     7429   +10     
=======================================
+ Hits        35337    35367   +30     
- Misses       4642     4644    +2     
  Partials      600      600           
Files Changed Coverage Δ
tests/test_pickle.py 99.64% <ø> (ø)
devito/passes/clusters/factorization.py 94.49% <75.00%> (+0.10%) ⬆️
devito/types/sparse.py 88.67% <97.43%> (-0.49%) ⬇️
devito/symbolics/queries.py 42.36% <100.00%> (ø)
devito/types/dense.py 93.89% <100.00%> (-0.06%) ⬇️
tests/test_dse.py 99.80% <100.00%> (+<0.01%) ⬆️
tests/test_interpolation.py 96.93% <100.00%> (+<0.01%) ⬆️
tests/test_sparse.py 99.27% <100.00%> (ø)

... and 1 file with indirect coverage changes

📢 Have feedback on the report? Share it here.

@mloubout mloubout force-pushed the drop-sf-parent branch 6 times, most recently from 729b68e to 5fb05ab Compare September 8, 2023 01:55
for k, v in w_funcs.items()])
except AttributeError:
assert w_funcs == 0
if len(w_funcs) > 1:
Copy link
Contributor Author

@mloubout mloubout Sep 8, 2023

Choose a reason for hiding this comment

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

@FabioLuporini this does not fix the pickle reconstruction issue with (-1)* so left the _print_Mul in printer for the time being.

See
https://github.com/devitocodes/devito/actions/runs/6113350028/job/16592681861#step:9:457

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. Can you open an issue about that and refer to the failing test without the hack?


expr = sf.inject(m, Float(1.))
sf.data.fill(1.)
expr = sf.inject(m, sf)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is actually a consequence of removing parent as injecting 1 akes an operator that does not process sf anymore as an argument as it was only processed through the coordinates as a parent

Copy link
Contributor

@FabioLuporini FabioLuporini left a comment

Choose a reason for hiding this comment

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

Thanks, uncontroversial

for k, v in w_funcs.items()])
except AttributeError:
assert w_funcs == 0
if len(w_funcs) > 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

OK. Can you open an issue about that and refer to the failing test without the hack?

@mloubout mloubout merged commit 0d4c099 into master Sep 8, 2023
32 checks passed
@mloubout mloubout deleted the drop-sf-parent branch September 8, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API api (symbolics, types, ...) compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SubFunction. circular dependency
3 participants