Add pqi_pqo_files to tests

[SuixiongTay]

Summary

Added pqi and pqo files for pure water (25 °C and pH = 7).

Todos

• Test case: Pure water

PHREEQC input:

SOLUTION 0
 temp 25.00
 pH 7.00
END

• Test case: Sophisticated solution (ex. pH, pE, species, volume)

PHREEQC input:

SOLUTION 0
 temp 25.00
 pH 7.00
 pe 4.0
 Ca 3.0 # total dissolved Ca
 Na 1.0
 Alkalinity 3.8 # mmol charge / kgw
 S 1.0 # total sulfur, mainly sulfate, S(6), at pe = 4
 N(5) 0.2 # nitrogen in the form of nitrate, N(5)
 Cl 1.0
 water 1 # kg water, default = 1 kg
 density 1
END

[codecov]

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #289   +/-   ##
=======================================
  Coverage   84.89%   84.89%           
=======================================
  Files           9        9           
  Lines        1476     1476           
  Branches      257      257           
=======================================
  Hits         1253     1253           
  Misses        193      193           
  Partials       30       30           

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

[rkingsbury]

Off to a good start @SuixiongTay ; thanks! Can you rename the folder to tests/files_phreeqc please?

[rkingsbury]

Is this line necessary? Since it points to a local directory (C:\...) it's not going to be useful (or readable) by our wrapper). Same goes for the other input files. If omitted, does PHREEQC just use its default database?

[SuixiongTay]

Let's cross validate the current DATABASE directory structure with Vineet's IPHREEQC schema to check. Tagging @vineetbansal

We could also specify the parser to skip entries containing the keyword DATABASE. Ideally, the test will check the following lines in the pqo output to validate the initial solution block:

------------------
Reading data base.
------------------

------------------------------------
Reading input data for simulation 1.
------------------------------------

-------------------------------------------
Beginning of initial solution calculations.
-------------------------------------------

------------------
End of simulation.
------------------

And we can remove the below:

   Input file: C:\Users\...
  Output file: C:\Users\...
Database file: C:\Program Files (x86)\USGS\Phreeqc Interactive 3.7.3-15968\database\phreeqc.dat
------------------------------------
Reading input data for simulation 2.
------------------------------------

-------------------------------
End of Run after 0.084 Seconds.
-------------------------------

What do you think? @rkingsbury

[rkingsbury]

Agreed; that sounds like a good approach.

[rkingsbury]

I'll go ahead and merge and we can clean up the DATABASE lines later if it turns out they aren't needed.

[vineetbansal]

Sorry I'm getting to this kinda late, after its been merged.

I thought this suggestion of including pqi/pqo files was towards our wrapper repository - pyphreeqc. That way when we added a (private) to_string method to our wrapper, we could check against these outputs.

Note that a to_string method is not useful for clients of pyphreeqc, and in fact dangerous, because it encourages custom parsing logic from the user, which can be error prone and goes against having a robust wrapper anyway. So I wasn't planning to have it in our API (to justify these "pass-through" tests in pyEQL). But if @rkingsbury or @SuixiongTay thinks they're useful, I can make it public (once implemented).

Also, in that case - if the ability to run strings and get strings back is what we're happy with, we may already be almost done with the wrapper :)

Or perhaps this in anticipation of pyphreeqc itself becoming part of pyEQL?

[rkingsbury]

I thought this suggestion of including pqi/pqo files was towards our wrapper repository - pyphreeqc. That way when we added a (private) to_string method to our wrapper, we could check against these outputs.

Oh, sorry for the misunderstanding there. We could do that too (or instead) if helpful.

Or perhaps this in anticipation of pyphreeqc itself becoming part of pyEQL?

This is probably the original source of my confusion. I had always envisioned the wrapper to become part of pyEQL itself (or at least, to be distributed with pyEQL and not something a user would ever use standalone). That doesn't necessarily mean it has to live inside the pyEQL repo though if there are benefits to keeping it separate. That's probably a bigger discussion best had next time we meet? But for now, if it helps you to put the test files inside pyphreeqc, let's do it.

Note that a to_string method is not useful for clients of pyphreeqc, and in fact dangerous, because it encourages custom parsing logic from the user, which can be error prone and goes against having a robust wrapper anyway. So I wasn't planning to have it in our API (to justify these "pass-through" tests in pyEQL). But if @rkingsbury or @SuixiongTay thinks they're useful, I can make it public (once implemented).

Hmmm, this is a good point. I was thinking that a Solution.to_phreeqc method (which basically creates the input file / string for PHREEQC) would be good to have for interoperability, because I expect many users of pyEQL will have come from, or still want to use, PHREEQC from time to time. So I was only thinking that a user would use it only to generate input files. In my view, your concern about custom parsing logic adds another reason why the string-generating method should be private (e.g., _to_phreeqc) and only be called inside Solution.to_file(), as I suggested in #288 .

Also, in that case - if the ability to run strings and get strings back is what we're happy with, we may already be almost done with the wrapper :)

Yeah, I think the first step of mapping Solution -> PHREEQC input string is nearly there (pending completion of _to_phreeqc() with thorough testing.

For the second step - "getting strings back" - I want the wrapper to have lightweight indexing methods for retrieving specific properties from the returned string / Array so that we don't have to manually figure out rows and columns. E.g., I'd like to be able to call something like Wrapper.get_activity('Na+') and have it give me the right number for a particular solute.