Changeset d8b4c5a in sasmodels

Timestamp:

Sep 26, 2018 3:45:11 PM (6 years ago)

Author:

butler

Branches:

master, core_shell_microgels, magnetic_model, ticket-1257-vesicle-product, ticket_1156, ticket_1265_superball, ticket_822_more_unit_tests

Children:

9235fc3

Parents:

2effdf6

Message:

Fix typo, clarify validation text and add tests

Addresses PR comments: removed redundant information in Iq function: doc
string replaced as suggested with text pointing to table and remove
default values on each parameter in the function call.
Also added verbiage to validation section to hopefully clarify what was
happening and what was done, fixed typo, and added back unit tests with
salt along with comment that these are "self-validated" and need proper
tests at some point in the future

File:

• sasmodels/models/be_polyelectrolyte.py (modified) (4 diffs)

sasmodels/models/be_polyelectrolyte.py

@@ -61 +61 @@
 history of this code goes back to a 1998 implementation. It was recently noted
 that in that implementation, while both the polymer concentration and salt
-concentration were converted to units of 1/|Ang|, only the converted polymer
-concentration was used in the calculation while the unconverted salt
-concentration was used.  This was carried through to sasmodles today (except
-that the conversion equation for the salt concentration was dropped somewhere
-along the line). Simple dimensional analysis of the calculation shows that the
+concentration were converted from experimental units of mol/L to more
+dimensionally useful units of 1/|Ang|^3, only the converted version of the
+polymer concentration was actually being used in the calculation while the
+unconverted salt concentration (still in units of mol/L) was being used.  This
+was carried through to sasmodels today (except that the line of code converting
+the salt concentration to the new units was dropped somewhere along the line).
+Simple dimensional analysis of the calculation shows that it is in fact the
 converted salt concentration must be used and the original code suggests that
 was the intention.  We therefore believe this is now correct.  Once better
@@ -119 +121 @@
 
 def Iq(q,
-       contrast_factor=10.0,
-       bjerrum_length=7.1,
-       virial_param=12.0,
-       monomer_length=10.0,
-       salt_concentration=0.0,
-       ionization_degree=0.05,
-       polymer_concentration=0.7):
+       contrast_factor,
+       bjerrum_length,
+       virial_param,
+       monomer_length,
+       salt_concentration,
+       ionization_degree,
+       polymer_concentration):
     """
-    :param q:                     Input q-value
-    :param contrast_factor:       Contrast factor of the polymer
-    :param bjerrum_length:        Bjerrum length
-    :param virial_param:          Virial parameter
-    :param monomer_length:        Monomer length
-    :param salt_concentration:    Concentration of monovalent salt
-    :param ionization_degree:     Degree of ionization
-    :param polymer_concentration: Polymer molar concentration
-    :return:                      1-D intensity
+    :params: see parameter table
+    :return: 1-D form factor for polyelectrolytes in low salt
+    
+    parameter names, units, default values, and behavior (volume, sld etc) are
+    defined in the parameter table.  The concentrations are converted from
+    experimental mol/L to dimensionaly useful 1/A3 in first two lines
     """
 
@@ -192 +191 @@
 
     # Accuracy tests based on content in test/utest_other_models.py
+    # Note that these should some day be validated beyond this self validation
+    # (circular reasoning). -- i.e. the "good value," at least for those with
+    # non zero salt concentrations, were obtained by running the current
+    # model in SasView and copying the appropriate result here.
+    #    PDB -- Sep 26, 2018
     [{'contrast_factor':       10.0,
       'bjerrum_length':         7.1,
@@ -202 +206 @@
      }, 0.001, 0.0948379],
 
-    #Comment out rest of tests as they use non zero salt concentrations. With
-    #the new code the results will change. We can just take the answer the code
-    #gives and call it correct but not sure that is appropriate.  May however
-    #be the best we can do? How were these generated in the first place?
-    #In the meantime comment them out. 
-    #Additional tests with larger range of parameters
-#    [{'contrast_factor':       10.0,
-#      'bjerrum_length':       100.0,
-#      'virial_param':           3.0,
-#      'monomer_length':         1.0,
-#      'salt_concentration':    10.0,
-#      'ionization_degree':      2.0,
-#      'polymer_concentration': 10.0,
-#      'background':             0.0,
-#     }, 0.1, -3.75693800588],
-
-#    [{'contrast_factor':       10.0,
-#      'bjerrum_length':       100.0,
-#      'virial_param':           3.0,
-#      'monomer_length':         1.0,
-#      'salt_concentration':    10.0,
-#      'ionization_degree':      2.0,
-#      'polymer_concentration': 10.0,
-#      'background':           100.0
-#     }, 5.0, 100.029142149],
-#
-#    [{'contrast_factor':     100.0,
-#      'bjerrum_length':       10.0,
-#      'virial_param':        180.0,
-#      'monomer_length':        1.0,
-#      'salt_concentration':    0.1,
-#      'ionization_degree':     0.5,
-#      'polymer_concentration': 0.1,
-#      'background':             0.0,
-#     }, 200., 1.80664667511e-06],
+    [{'contrast_factor':       10.0,
+      'bjerrum_length':       100.0,
+      'virial_param':           3.0,
+      'monomer_length':         5.0,
+      'salt_concentration':     1.0,
+      'ionization_degree':      0.1,
+      'polymer_concentration':  1.0,
+      'background':             0.0,
+     }, 0.1, 0.253469484],
+
+    [{'contrast_factor':       10.0,
+      'bjerrum_length':       100.0,
+      'virial_param':           3.0,
+      'monomer_length':         5.0,
+      'salt_concentration':     1.0,
+      'ionization_degree':      0.1,
+      'polymer_concentration':  1.0,
+      'background':             1.0,
+     }, 0.05, 1.738358122],
+
+    [{'contrast_factor':     100.0,
+      'bjerrum_length':       10.0,
+      'virial_param':         12.0,
+      'monomer_length':       10.0,
+      'salt_concentration':    0.1,
+      'ionization_degree':     0.5,
+      'polymer_concentration': 0.1,
+      'background':           0.01,
+     }, 0.5, 0.012881893],
     ]