search

jrbourbeau / pyunfold

Iterative unfolding for Python
https://jrbourbeau.github.io/pyunfold/
MIT License
26 stars 13 forks source link

Adds 2D response matrix check #90

Closed jrbourbeau closed 6 years ago

jrbourbeau commented 6 years ago

This PR adds a check that input response matrices are 2-dimensional. Now a more meaningful error message will be raised when a non-2D response matrix is used.

Ref: #89

zhampel commented 6 years ago

Hey @jrbourbeau, in the new test test_mixer_2d_response_check() in test_mix.py that you've added, it declares a new response matrix, but I see at the top of that script that this has arguments num_effects, num_causes, num_effects. Can you please explain why the extra num_effects is needed? Ideally, there is one set of effects and one can add any number of causes to the response matrix.

jrbourbeau commented 6 years ago

That test is checking that an error is raised if the input response matrix is not 2-dimensional. The extra num_effects is just to specify a certain size for the third dimension of the (bad) response matrix. Instead of 

response = 1 + np.random.rand(num_effects, num_causes, num_effects)

I can add a comment and switch to something like 

# Construct a response matrix that is 3-dimensional (should raise an error)
response_bad = 1 + np.random.rand(num_effects, num_causes, 1)

to make it more clear what the motive is for the test.

zhampel commented 6 years ago

@jrbourbeau Oh yes of course! Jet lag is killer, I wasn't thinking straight but indeed, you need more dims for this check! :) Thanks

Looks good to me especially with the renaming suffix '_bad'. Passes inspection 👍