Skip to content

Benchmarking visualization#399

Open
YitongPan1 wants to merge 3 commits into
mainfrom
YitongPan1-patch-4
Open

Benchmarking visualization#399
YitongPan1 wants to merge 3 commits into
mainfrom
YitongPan1-patch-4

Conversation

@YitongPan1
Copy link
Copy Markdown
Collaborator

@YitongPan1 YitongPan1 commented May 14, 2026

Summary

Major changes:

supports #252

  • feature 1: Integrate CRC dataset into package structure under pyEQL/database;
  • feature 2: Add benchmarking calculations for pyEQL properties (activity coefficient, conductivity);
  • feature 3: Enable visualization of experimental CRC data against pyEQL simulation results;
  • feature 4: Add usage example for running benchmark function;

Example usage:

benchmark(cond, ["Mg[+2]", "Cl[-1]"], "conductivity")
image

Todos

If this is work in progress, what else needs to be done?

  • limitation 1: Current dataset only supports a limited set of electrolytes including NaCl, KCl, CaCl2, MgCl2, Na2SO4, K2SO4, and MgSO4;
  • Improvement 1: Add error handling for unsupported ion combinations and missing data cases.

Checklist

  • Google format doc strings added.
  • Code linted with ruff. (For guidance in fixing rule violates, see rule list)
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • I have run the tests locally and they passed.

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

@YitongPan1 YitongPan1 marked this pull request as ready for review May 14, 2026 17:32
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.21%. Comparing base (2203ae5) to head (82a5976).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #399   +/-   ##
=======================================
  Coverage   86.21%   86.21%           
=======================================
  Files          14       14           
  Lines        1842     1842           
  Branches      318      318           
=======================================
  Hits         1588     1588           
  Misses        209      209           
  Partials       45       45           

☔ 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.

Copy link
Copy Markdown
Member

@rkingsbury rkingsbury left a comment

Choose a reason for hiding this comment

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

Thank you @YitongPan1 , it will be great to have a built-in way to generate these types of plots. Please modify a few things:

  1. create a new folder benchmarks under /src/pyEQL and move all your data files into it
  2. rename Benchmarking_plot benchmark(.py) and keep it in the src/pyEQL directory. (another python convention - module names are always lowercase)
  3. Include bibliographic information for the specific CRC tables you used in the data files. You can probably do this as the first line of your .CSVs, which you can then tell pandas to skip when loading the file
  4. Add a docstring for the user-facing function benchmark
  5. prefix any private functions (not intended for direct use by end user) with a _. So filter_df -> _filter_df. (Yet another python convention)
  6. Add a page under docs that shows plots for each of the salts you have and includes the reference information. Make a separate subfolder called plots under docs to save the images. (This item can be a separate PR if you prefer)

@rkingsbury
Copy link
Copy Markdown
Member

Also remember to lint by running pre-commit - this would likely have auto-corrected or flagged some of the issues I raised like missing docstrings.

@SuixiongTay SuixiongTay mentioned this pull request May 15, 2026
@rkingsbury rkingsbury mentioned this pull request May 28, 2026
16 tasks
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.

2 participants