search

inductiva / wind-tunnel

Example of a virtual Wind Tunnel implementation with Inductiva API.
1 stars 2 forks source link

Improve reverse transformation #37

Open rcvalerio opened 2 months ago

rcvalerio commented 2 months ago

Currently we return the transformations during windtunnel.set_object() and its the user responsibility to apply the reverse transformations to the outputs (e.g streamlines, pressure map..).

I believe it is better to add an optional transformations argument to the WindTunnelOutputs class. This allows us to have a optional argument "apply_reverse_transformations: bool" to output functions such as get_streamlines(apply_reverse_transformations=True).

If the transformations had not been passed to the WindTunnelOutputs class then an exception would be raised when the user tries to use the argument apply_reverse_transformations.

What do you think?

luispcunha commented 2 months ago

I think it makes sense.

luispcunha commented 2 months ago

If the transformations had not been passed to the WindTunnelOutputs class then an exception would be raised when the user tries to use the argument apply_reverse_transformations.

In which scenario would this happen? The WindtunnelOutputs object is created by the "windtunnel", right?

rcvalerio commented 2 months ago

No, the WindTunnel and WindtunnelOutputs classes are totally decoupled. WindTunnel is used to submit simulations while WindtunnelOutputs is used to obtain the results.

Having the transformations be required in the constructor adds unnecessary complexity to our package because we don't always need to apply reverse transformations, for example, when normalizing and centering are disabled. If it was a required parameter the user would have to save the transformation that we return and pass it to the WindtunnelOutputs for no reason.

luispcunha commented 2 months ago

Then I'm not sure, because if the user has to pass those transformations, then it doesn't make sense to call it "reverse_transformation" since the user could pass arbitrary transformations.