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

More consistent evaluation environment #161

Merged
merged 8 commits into from
Mar 15, 2024
Merged

More consistent evaluation environment #161

merged 8 commits into from
Mar 15, 2024

Conversation

sunxd3
Copy link
Member

@sunxd3 sunxd3 commented Mar 15, 2024

The PR brings the following changes to the code:

  • An eval_env is created early in the pipeline (right before transformed data are computed, after array sizes are known). eval_env will be used across the entire compilation and logp computation.
  • merge_with_coalescence is removed, it is slow and single-purpose
  • Some of the code used to assume that, variables not in env are non-data variables, now they need to check if is_resolved
  • find_variables_on_lhs is also removed -- the Var interface will be phased out
  • some variable namings are made more coherent

@sunxd3 sunxd3 changed the title Remove merge_with_coalescence More consistent evaluation environment Mar 15, 2024
test/runtests.jl Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

Attention: Patch coverage is 96.61836% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 81.13%. Comparing base (f9979da) to head (4dbee9c).
Report is 1 commits behind head on master.

Files Patch % Lines
src/compiler_pass.jl 96.39% 4 Missing ⚠️
src/variable_types.jl 80.00% 2 Missing ⚠️
src/utils.jl 97.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #161      +/-   ##
==========================================
- Coverage   82.27%   81.13%   -1.14%     
==========================================
  Files          21       21              
  Lines        1991     1945      -46     
==========================================
- Hits         1638     1578      -60     
- Misses        353      367      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sunxd3
Copy link
Member Author

sunxd3 commented Mar 15, 2024

Before:

Dataset Stage Avg Time Min Time Max Time Memory Usage
lsat CollectVariables 91.656 ms 88.738 ms 93.081 ms 31.44 MiB
DataTransformation 198.143 ms 196.257 ms 203.257 ms 169.02 MiB
NodeFunctions 894.018 ms 856.648 ms 1.162 s 263.36 MiB
inhalers CollectVariables 53.827 ms 53.067 ms 62.738 ms 18.42 MiB
DataTransformation 200.013 ms 197.998 ms 203.605 ms 209.34 MiB
NodeFunctions 539.151 ms 528.337 ms 814.248 ms 182.89 MiB
leuk CollectVariables 17.154 ms 16.888 ms 21.298 ms 6.26 MiB
DataTransformation 77.602 ms 74.918 ms 84.727 ms 92.25 MiB
NodeFunctions 130.587 ms 129.419 ms 134.164 ms 43.82 MiB
epil CollectVariables 5.750 ms 5.655 ms 12.651 ms 2.10 MiB
DataTransformation 28.721 ms 26.649 ms 35.697 ms 29.90 MiB
NodeFunctions 111.960 ms 110.671 ms 120.250 ms 44.31 MiB

After:

Dataset Stage Avg Time Min Time Max Time Memory Usage
lsat CollectVariables 94.551 ms 90.920 ms 104.237 ms 31.44 MiB
DataTransformation 126.559 ms 124.153 ms 136.073 ms 71.57 MiB
NodeFunctions 710.050 ms 669.937 ms 866.867 ms 227.15 MiB
inhalers CollectVariables 54.531 ms 53.561 ms 65.025 ms 18.41 MiB
DataTransformation 108.058 ms 106.586 ms 118.391 ms 84.91 MiB
NodeFunctions 492.562 ms 482.581 ms 675.718 ms 179.89 MiB
leuk CollectVariables 17.755 ms 17.526 ms 22.863 ms 6.56 MiB
DataTransformation 57.119 ms 55.818 ms 70.786 ms 63.32 MiB
NodeFunctions 103.667 ms 100.935 ms 106.005 ms 39.02 MiB
epil CollectVariables 5.870 ms 5.789 ms 10.047 ms 2.09 MiB
DataTransformation 22.214 ms 21.697 ms 32.339 ms 20.32 MiB
NodeFunctions 99.995 ms 98.439 ms 110.111 ms 44.69 MiB

@sunxd3 sunxd3 merged commit 42484fe into master Mar 15, 2024
14 of 15 checks passed
@sunxd3 sunxd3 deleted the sunxd/remove_merge branch March 15, 2024 12:34
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.

1 participant