Skip to content

3310 optimise dependency tool#3311

Open
hiker wants to merge 2 commits intomasterfrom
3310_optimise_dependency_tool
Open

3310 optimise dependency tool#3311
hiker wants to merge 2 commits intomasterfrom
3310_optimise_dependency_tool

Conversation

@hiker
Copy link
Collaborator

@hiker hiker commented Jan 30, 2026

Delivers significant speedup in the dependency analysis for files that have many array indices that just use a simple reference as index (a(i,j), not a(i+1, h)) by avoiding calling sympy in these simple cases.

In two ukca files some loops are 30 to over 40 times faster (measurements in #3310).

@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (59f796e) to head (fe1b69b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3311   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files         380      380           
  Lines       53949    53953    +4     
=======================================
+ Hits        53927    53931    +4     
  Misses         22       22           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hiker
Copy link
Collaborator Author

hiker commented Jan 30, 2026

IT passed as well, a minor change with big impact (on selected few files ;) )

@hiker
Copy link
Collaborator Author

hiker commented Feb 2, 2026

I am taking this back for now. While one case (above) seems consistent, I re-did some measurements, and I am getting some inconclusive results ... it could be that the mn416-branch that I used missed some updates to the dependency_tools?

@hiker
Copy link
Collaborator Author

hiker commented Feb 3, 2026

I've verified that the 'speedup' of the DA on master was caused because the references were not resolved (turned up as unknown, so DA aborted early). Resolving the symbol fixes the 'speedup' (which marked the loop as not parallelisable, even though it is), it is then slow again with the correct result, and this patch fixes it:

master:
ukca_um_strat_photol_mod.F90	2	Execution time loop i:	14.372405	True	
ukca_um_strat_photol_mod.F90	3	Execution time loop j:	10.499314	True	
ukca_aerod.F90	                1	Execution time loop j:	35.180508	False	

3310:
ukca_um_strat_photol_mod.F90	2	Execution time loop i:	0.155515	True	
ukca_um_strat_photol_mod.F90	3	Execution time loop j:	0.132040	True	
ukca_aerod.F90	                1	Execution time loop j:	0.695516	False	

@hiker
Copy link
Collaborator Author

hiker commented Feb 3, 2026

Looks like no changes, so IT do not run again. Ready to go. Note that there are still some outliers when running the DA on ukca. I've added some additional potential improvements to #3183 (#3183 (comment))

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant