Skip to content

Adjust limits for Guinier-Porod model.#667

Merged
butlerpd merged 1 commit intomasterfrom
fix-limits-for-guinier-porod
Jan 13, 2026
Merged

Adjust limits for Guinier-Porod model.#667
butlerpd merged 1 commit intomasterfrom
fix-limits-for-guinier-porod

Conversation

@pkienzle
Copy link
Copy Markdown
Contributor

@pkienzle pkienzle commented Sep 5, 2025

Limit s to the range [0, 3].

Fix limiting values for I(q) when Rg or Porod exponent m go to zero. Add limiting value for s=3.

@gonzalezma gonzalezma self-requested a review November 17, 2025 17:06
Copy link
Copy Markdown
Contributor

@gonzalezma gonzalezma left a comment

Choose a reason for hiding this comment

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

All clear.
Perhaps remove unneeded parenthesis around exp(-ms) * (n*ms/rg**2)**ms?

@pkienzle
Copy link
Copy Markdown
Contributor Author

Perhaps remove unneeded parenthesis around exp(-ms) * (n*ms/rg**2)**ms?

The parenthesis are there to force python to evaluate the expression as vector * (scalar * scalar) rather than (vector * scalar) * scalar. The second form costs twice as much.

@gonzalezma
Copy link
Copy Markdown
Contributor

Thanks. I missed that.

@krzywon krzywon requested review from smk78 and yunliu01 December 16, 2025 14:41
@krzywon
Copy link
Copy Markdown
Collaborator

krzywon commented Dec 16, 2025

@yunliu01 and @smk78 - A discussion from todays meeting asked if these limits will effect any science cases. The model itself says any value outside the range of 0-3 is unstable, but that doesn't mean there aren't any science cases where that isn't true. Please check when you are able.

@smk78
Copy link
Copy Markdown
Contributor

smk78 commented Dec 28, 2025

I will be honest: I have very little experience of this model.

But I note the text under Eq 4 in the Hammouda paper. If s were >3 then the 'dimensionality parameter' would be negative and that instinctively seems bad?

I also note @mdoucet imposed an upper limit of 3 when he wrote the GP fit function for Mantid (https://github.com/mantidproject/mantid/blob/400d30c758e6bd9f5441c1e3e5287f6c9164785e/Framework/PythonInterface/plugins/functions/GuinierPorod.py).

So on this basis I see no reason not to approve.

Copy link
Copy Markdown
Contributor

@smk78 smk78 left a comment

Choose a reason for hiding this comment

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

Have not reviewed the code, but approve the limits imposed on parameter s.

@butlerpd butlerpd merged commit 206ba38 into master Jan 13, 2026
20 checks passed
@butlerpd butlerpd deleted the fix-limits-for-guinier-porod branch January 13, 2026 15:04
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.

5 participants