Opened 4 months ago

Closed 3 months ago

#1155 closed defect (fixed)

BE Polyelectrolyte errors

Reported by: butler Owned by: butler
Priority: blocker Milestone: SasView 4.2.0
Component: SasView Keywords:
Cc: Work Package: SasView Bug Fixing

Description (last modified by butler)

Sylvain Prevost sent the following in November 2017 (subsequently lost and refound) regarding problems with this model. He believes there are two problems:

  • In the python code: concentration is rescaled from mol/dm3 to 1/Å3 only for monomer concentration, not for salt concentration. Further, polymer concentration is probably a misnomer for monomeric units concentration
  • In the docs:
    • The definition of r02 is wrong, { b/√(48 π Lb) } should be in the numerator, not the denominator: r02 = b / [ α √(Ca 48 π Lb) ]
    • in the main equation, it should read r04, not r02
    • in the docs, the conversion of concentration from mol/dm3 to 1/Å3 should be explicitly indicated before any equation
    • for consistency, in the main equation, it should read (q2 - 12 h/NA Ca / b2) where NA is Avogadro's constant (as h is supposedly in Å3/mol, and assuming Ca has been converted to 1/Å3

Change History (5)

comment:1 Changed 4 months ago by butler

  • Description modified (diff)

comment:2 Changed 4 months ago by butler

going through the math (is all in python) it is clear that indeed:

  • r02 is calculated as described above - documentation needs to be fixed
  • The code calculates r_squared**2 which indeed is r04 - documentation should be fixed
  • the virial parameter h is supposed to be given in Å3 NOT L/mol - equation in docs is ok BUT parameter units needs to be changed
  • Indeed, units would seem to indicate that the salt concentration should also be converted to 1/Å3. Looking at the IGOR code this was originally drawn from, it is interesting to note that indeed Ca and Csa are both calculated … but then only Ca is used as here. This may be a mistake that goes way back. This needs further checking.

comment:3 Changed 4 months ago by butler

  • Owner set to butler
  • Status changed from new to assigned

comment:4 Changed 3 months ago by butler

  • Priority changed from major to blocker

At this point all remaining tickets should either be blockers or critical. Critical would be those that do not stop the actual release but need to be dealt with soon after such as updating the marketplace. Tickets such as this are blockers in that if not fixed the docs need to have an appropriate warning in them for this release. Once that is fixed the ticket moves to 4.3 if it is not closed.

comment:5 Changed 3 months ago by GitHub <noreply@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In a5bcd61ecf04134efa21afe21e10363f33f72063/sasmodels:

Merge pull request #82 from SasView?/ticket-1155BE_PolyElectrolyte

Fix docs and code for

Closes #1155
Will open a new ticket that validation is still required.

Note: See TracTickets for help on using tickets.