Skip to content

WIP: Adding Electrical Conductivity Methods#73

Closed
sammykojo wants to merge 33 commits intovbr-calc:mainfrom
sammykojo:cond
Closed

WIP: Adding Electrical Conductivity Methods#73
sammykojo wants to merge 33 commits intovbr-calc:mainfrom
sammykojo:cond

Conversation

@sammykojo
Copy link

No description provided.

Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
@chrishavlin chrishavlin added the enhancement New feature or request label Nov 21, 2023
@chrishavlin
Copy link
Member

chrishavlin commented Nov 21, 2023

Here's a list of things we'll want to do eventually before merging:

  • add a description to the pull request
  • add an example to Projects/vbr_core_examples
  • add a page to the full documentation describing methods
  • add to the automated test suite
  • add units metadata

Copy link
Member

@chrishavlin chrishavlin left a comment

Choose a reason for hiding this comment

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

I got this tied into the whole VBRc automated backend locally, but there are a couple fixes needed to the files you've already changed before I send along the other changes.

@chrishavlin
Copy link
Member

I'm going to try to push up the changes need to get this picked up by the automated methods in the Spine functions...

@chrishavlin
Copy link
Member

chrishavlin commented Nov 21, 2023

Ok! @sammykojo I just pushed up a bunch of changes to your branch that get this working with all of the automated methods that the VBRc uses to call the functions. I also added an initial example, Projects/vbr_core_examples/CB_014_electrical_conductivity.m . Currently it errors, but it is properly picking up the new methods and functions. The errors should be fixed after you incorporate my latest review comments.

@@ -0,0 +1,66 @@
function [ VBR ] = yosh2009( VBR )
Copy link
Member

Choose a reason for hiding this comment

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

Oh -- one more suggestion while you're updating all these functions, I'd prefer if they all had a common prefix. Maybe cond_yosh2009.m, cond_wang2006.m, etc., to help organize the functions folder. This change can wait until after things are initially working...

sammykojo and others added 11 commits December 4, 2023 10:13
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Fix for nested loop structure when creating field names in Params_Electric (second-preferred method)
State Variable in, fix for DK2014 (add units as a subscript to Pressure & Temperature variable name)
State Variable in fix for SEO3 (addition of unit as subscript to Temperature variable name)
State Variable in fix for UHO2014 (addition of units as subscript to Pressure & Temperature variable names )
State Variable in fix for poe2010 (addition of units as subscript to Pressure and Temperature variable names)
State Variable in fix for sun2019 (addition of units as subscripts for Temperature & Pressure variable names)
State Variable in for wang2006 (addition of units as subscript for Temperature & Pressure variable names)
State Variable in fix for yosh2009 (addition of units as subscript for Pressure & Temperature variable names)
Copy link
Member

@chrishavlin chrishavlin left a comment

Choose a reason for hiding this comment

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

added some suggestions that will fix yosh2009 and wang2006 -- you'll have to go through the other functions to change them in the same way.

sammykojo and others added 4 commits December 4, 2023 19:52
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
Co-authored-by: Chris Havlin <chris.havlin@gmail.com>
@chrishavlin
Copy link
Member

chrishavlin commented Dec 5, 2023

Hey @sammykojo , with the latest changes, the example I added to this PR now runs with yosh2009 without error (the example at this path: Projects/vbr_core_examples/CB_014_electrical_conductivity.m). Something is going wrong in the calculation though -- after running that example, the sig fields in VBR.out.electric.yosh2009 are all inf or absurdly large. My first guess would be an issue with units or maybe a sign error in an exponential.

exclusion of Hydrogen diffusion experiments
Exclusion of H-D diffusion experiments because of inconsistencies (Novella, 2017 & Sun, 2019)

Exclusion of Dai and Karato, 2014 because of experimental design (done at single hydration value)

Inclusion of Jones et al, 2012

Correction of Poe 2010 using Jones 2012 wt% Ch2o in stead of original parameters for ppm Ch2o

Current Electrical Conductivity functions (Esigs):
esig.yosh2009 = yosh2009(T, Ch2o, P);
esig.SEO3 = SEO3(T, Ch2o, P);
esig.poe2010 = poe2010(T, Ch2o, P);
esig.sun2019 = sun2019(T, Ch2o, P);
esig.wang2006 = wang2006(T, Ch2o, P);
esig.UHO2014 = UHO2014(T, Ch2o, P);
esig.jones2012 = jones2012(T, Ch2o, P);

*All corresponding individual functions have been updated
@chrishavlin
Copy link
Member

Great to see the activity, @sammykojo ! let me know when you're ready for me to take another look.

sammykojo and others added 3 commits March 8, 2024 14:56
Correct grammar errors 

Update Possible_methods list
@chrishavlin
Copy link
Member

superseded by #92

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.

2 participants