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

Add profiling & benchmarks for Indexset and Parameter #148

Closed
wants to merge 7 commits into from

Conversation

glatterf42
Copy link
Member

@glatterf42 glatterf42 commented Jan 13, 2025

In order to evaluate the benefit of #143, @meksor asked me to write some profiling and benchmark tests for parameters.add(data). This PR does exactly that and a bit more, so there are a few things to keep in mind:

  • I started with writing tests for Indexset because Indexset already had its DB model normalized (Normalize IndexSet.data DB storage #122). This might be helpful because we can already compare with this PR alone how the Indexset model compares to converting data to dicts and storing them as JSON. This comparison is not accurate, though, so my next task will be to duplicate Normalize optimization.Table DB storage #143 for Parameter. (We are interested in Parameter instead of Table because that needs the UPSERT functionality, the benchmark of which triggered this whole procedure.) On top of that branch, I can include exactly the same tests we have here for a proper comparison.
  • This PR contains the script that creates the test data. This should probably not be committed to main or at least requires more cleanup before being committed.
  • For now, some tests are not using the big data (because I only ensured that the tests are running locally, for which I didn't want to wait so long). For proper benchmarks runs, we may want to adapt this. And add some warmup-runs and iterations.
  • This PR also contains tests/fixtures/optimization/big/parameterdata.csv, which is too large for GitHub's liking. When I pushed the commit adding the file here, I received the following:
    remote: warning: See https://gh.io/lfs for more information.
    remote: warning: File tests/fixtures/optimization/big/parameterdata.csv is 98.55 MB; this is larger than GitHub's recommended maximum file size of 50.00 MB
    remote: warning: GH001: Large files detected. You may want to try Git Large File Storage - https://git-lfs.github.com.
    I'm not sure if it's preferrable to use git-lfs or create the test data dynamically.
  • This morning, I came across https://github.com/gazorby/fish-git-emojis?tab=readme-ov-file. I like the idea of enabling a quick overview of commits by using emojis and potentially enabling tools to run on commit messages, though I admit that "Optimization Profiling" is likely not the best scope to use. What do you think?

@glatterf42 glatterf42 added the enhancement New feature or request label Jan 13, 2025
@glatterf42 glatterf42 requested a review from meksor January 13, 2025 12:44
@glatterf42 glatterf42 self-assigned this Jan 13, 2025
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.9%. Comparing base (0deac5b) to head (3bf23f4).

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #148   +/-   ##
=====================================
  Coverage   86.9%   86.9%           
=====================================
  Files        230     230           
  Lines       8156    8156           
=====================================
  Hits        7095    7095           
  Misses      1061    1061           

@glatterf42 glatterf42 force-pushed the profile-optimization-db branch from 3bf23f4 to 7e3874e Compare January 16, 2025 12:28
@glatterf42 glatterf42 changed the base branch from main to fix/avoid-superfluous-db-calls-from-core January 16, 2025 13:19
Base automatically changed from fix/avoid-superfluous-db-calls-from-core to main January 16, 2025 14:07
@glatterf42
Copy link
Member Author

Superseded by #150.

@glatterf42 glatterf42 closed this Jan 16, 2025
@glatterf42 glatterf42 deleted the profile-optimization-db branch January 16, 2025 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant