Negative velocities #64
Conversation
| @@ -75,6 +75,40 @@ def test_single_point(): | |||
| assert error < 1e-10, "Numerical error too big" | |||
There was a problem hiding this comment.
As far as I can see blob_alignment is actually never tested. Could we include a specific test?
There was a problem hiding this comment.
Sure, the reason I didn't add was because I didn't find any test for the actual alignment implementation either (the current blob tests work both for alignment true and false, without changing expected values)
| @@ -23,6 +24,7 @@ def __init__( | |||
| t_drain: Union[float, NDArray], | |||
| prop_shape_parameters: dict = None, | |||
| perp_shape_parameters: dict = None, | |||
| blob_alignment: bool = True, | |||
There was a problem hiding this comment.
If we want to keep the blob alignment it would probably make sense to access it in the model class. What do you think?
There was a problem hiding this comment.
I had it there first. I changed it to the forcing class to minimize interface changes since I guess most people will not care whether their blobs are aligned or not. I can add it there, it is just to send it straight to the forcing constructor.
I am working with something that requires the velocities to be negative. Even though there are physical arguments to why this might not be the case, I believe the code should be able to handle negative velocities. I also need that to test my estimation methods in the case of negative radial velocities.
I also added the blob_alignment parameter to keep me from going insane. I can remove it if you believe if is not necessary.
The compute_start_stop method from the model is getting a bit complicated and hard to read, we should maybe consider a refactor.