Skip to content

Benchmarking visualization - #399

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

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

Conversation

@YitongPan1

@YitongPan1 YitongPan1 commented May 14, 2026

Copy link
Copy Markdown
Collaborator

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

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.54%. Comparing base (dd1be5f) to head (074d8c0).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #399   +/-   ##
=======================================
  Coverage   87.54%   87.54%           
=======================================
  Files          14       14           
  Lines        1951     1951           
  Branches      340      340           
=======================================
  Hits         1708     1708           
  Misses        192      192           
  Partials       51       51           

☔ View full report in Codecov by Harness.
📢 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.

@rkingsbury rkingsbury left a comment

Copy link
Copy Markdown
Member

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
@rkingsbury rkingsbury added this to the v1.6.0 milestone Jun 15, 2026
@rkingsbury rkingsbury removed this from the v1.6.0 milestone Jul 29, 2026
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