SasView / sasmodels

Package for calculation of small angle scattering models using OpenCL.
BSD 3-Clause "New" or "Revised" License
16 stars 28 forks source link

Re-disable buggy check #576

Closed lucas-wilkins closed 1 year ago

lucas-wilkins commented 1 year ago

Previous attempts to unbug the orientation viewer didn't work out, this re-disables the breaking check until the right solution can be found.

pkienzle commented 1 year ago

I would rather not silently ignore incorrect parameters to the model evaluator. If the caller can't be fixed for this release then throw in a deprecation notice that undefined parameters will raise an error in a future release.

lucas-wilkins commented 1 year ago

@pkienzle It's not ideal to disable checks, but was less ideal to merge something that broke the current sasview.

If you could tell me what the new way of doing things is, then it would be pretty easy to update on the sasview end.

The current version (in the branch where the updated parameter names) is something like:

model_info = load_model_info("parallelepiped")
model = build_model(model_info)
...
calculator = DirectModel(data, model)
calculator(
  length_a=a,
  length_b=b,
  length_c=c,
  ... [other stuff])

Using length_a instead of a stops the errors, but it also stops it from calculating anything at all, just returning a uniform field. If you know what's going on, let me know and I'll fix it, if you don't know, then maybe its best to disable the check until we can figure it out.

pkienzle commented 1 year ago

You have a teeny object of size [0.1 x 0.4 x 1.0] which leads to scattering < 3.6e-5 over your q mesh. You are adding a background of exp(-3) ≈ 0.05 then converting to a log-scaled colour mapped over [exp(-3), exp(10)] ≈ [0.05, 22000].

Multiplying your dimensions by 200 works pretty well, giving interesting patterns in the q range and peaks ≈280.

lucas-wilkins commented 1 year ago

I'm pretty sure they're the same values as in jitter.py, no? And it worked fine before. Is there something that has changed with the parameter scaling, as well as renaming them all?

pkienzle commented 1 year ago

I never tied the shape parameters to the drawn box size in jitter, hence the confusion.

A simple solution is to divide each dimension of the drawn shape by the max dimension of the calculated shape so the rectangle is drawn with unit length. That way you preserve the aspect ratio without drawing anything too large or too small.

lucas-wilkins commented 1 year ago

I think the default parameters just happened to be at least approximately the right shape for the lengths in the orientation viewer. I'll add something that takes a size in Angstroms, and normalises it to 1 along its longest length for the drawing part.

wpotrzebowski commented 1 year ago

@lucas-wilkins do we still need it? The orientation viewer seems to be working now.

lucas-wilkins commented 1 year ago

Close!

On Tue, 12 Sept 2023, 10:31 Wojciech Potrzebowski, @.***> wrote:

@lucas-wilkins https://github.com/lucas-wilkins do we still need it? The orientation viewer seems to be working now.

— Reply to this email directly, view it on GitHub https://github.com/SasView/sasmodels/pull/576#issuecomment-1715355838, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACKU4SX52U4C57FA6RZZ4NTX2ATVLANCNFSM6AAAAAA3SFIGUM . You are receiving this because you were mentioned.Message ID: @.***>