search

boutproject / boutdata

GNU Lesser General Public License v3.0
0 stars 2 forks source link

Performance improvements #101

Closed dschwoerer closed 4 months ago

dschwoerer commented 1 year ago

The first commit is the most important one. For me it reduced the time from many, many hours to around 20 minutes.

2e095a22bec1114720086f3838a0ed36352a3be5 reduces the combinatory explosion. I don't think we need for example check the different compression levels. Maybe we can remove a bit more, or less. Anyway, that should make the unit tests faster. Beyond reducing, I think we would ne some fake datafile interface, but that would probably be more effort then it is worth.

18b4d4b36db2a65438137188935710cdf55d0fd3 removes the --cov flag - which makes it slightly faster. Mostly I assume most people don't care that much about coverage. If at all, it make sense to compare coverage before and after a change, but then one actually needs to look at the output ...

I think we should merge the first commit, happy to discuss and/or revert the other two ...

dschwoerer commented 1 year ago

Just as an update, the first commit seems to be important for python3.12, it changes the run time of the tests from around 17 hours to normal times ...

dschwoerer commented 9 months ago

This was introduced here: https://github.com/boutproject/BOUT-dev/pull/1241

I think I have found the issue in the mean time, for some reason some back traces from some exceptions in netCDF do not get garbage collected, and thus the gc.collect gets extremely slow as there are millions of objects. So the original code is fine, and there is a different issue. If others are not seeing the slow-down, I am fine with keeping this as a fedora-only patch and try to find the real issue ...

ZedThree commented 4 months ago

Having a look at this again and profiling the tests, we spend a good fraction of the time in create_dump_files -- should be able to pull this out into a fixture and cache the files for each geometry. I'll have a stab at that next week

ZedThree commented 4 months ago

Changes now in #107