Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reader2D_converter error calculations are proper sketchy #45

Open
lucas-wilkins opened this issue Aug 17, 2023 · 8 comments
Open

reader2D_converter error calculations are proper sketchy #45

lucas-wilkins opened this issue Aug 17, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@lucas-wilkins
Copy link
Contributor

@ehewins was asking about this code, which appears to assume that errors are derived from shot noise.

    if data2d.err_data is None or np.any(data2d.err_data <= 0):
        new_err_data = np.sqrt(np.abs(new_data))

Looking at the use cases, such as loading .tiff files, shot noise seems like a pretty unsound assumption. Is there anything wrong with just not specifying errors when they are not known?

@lucas-wilkins lucas-wilkins added the bug Something isn't working label Aug 17, 2023
@butlerpd
Copy link
Member

Humm... Don't know this bit of code but in principle I am a very strong proponent for not inventing something that is not in the data. That just seems wrong?

However, if this is about the image converter (you mention TIFF?) that might be because it is meant to deal with raw data from lab SAXS? There was some discussion years ago about that and I don't remember them fully, though @smk78 may remember more? The gist as I recall was that we were getting a lot of pleas from lab SAXS users who had tiff files and no way to easily process their data (don't ask me why) to provide a way to get their image into SasView. They also then like to use the data operations to "reduce" their data I believe?

At the very least I would think a popup option in the converter asking the user if that is what is expected?

@lucas-wilkins
Copy link
Contributor Author

There's no way that the chosen errors even remotely represent the noise, is there?

@lucas-wilkins
Copy link
Contributor Author

@timsnow might also have some thoughts.

@butlerpd
Copy link
Member

? If the images is of raw counts as collected from the detector then would that not in fact be the uncertainty? I believe that is what all current reduction programs use.

Though you are starting to open a can of worms because as far as I know, almost all reduced data (processed) only contains those (counting statistics) errors appropriately propagated but ignore all of the many other systematics that contribute to the processed data -- but guessing that is not the question here?

@lucas-wilkins
Copy link
Contributor Author

If they were raw counts, then that would indeed be the right formula for shot noise.

I thought SAXS was dominated by other uncertainties though.

@butlerpd
Copy link
Member

🤐

@lucas-wilkins
Copy link
Contributor Author

In any case, I don't think it's right to assume that all images are of raw counts, and even if it was, the thing that makes that assumption should probably live in the image reader, not a "data converter" in a general location.

@butlerpd
Copy link
Member

I agree - we should not be making random assumptions about data we are ingesting. That is why I suggested it be an option that the user selects somehow.

However, I think there should be input from people with more knowledge about this. There may be reasons it was done that way (maybe it is clear somewhere that the feature is ONLY for raw counts?) Unfortunately @krzywon is out through next week. I think @gonzalezma may have been involved in writing part of that code? and, as I say, I think @smk78 was at least partially responsible for asking for the convert features and worked with those who were implementing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants