Let Model outputs be NumPy arrays

[sdrave]

As discussed, outputs of Models are made NumPy arrays instead of VectorArrays to facilitate interfaces with pyMOR Models from user code / external code.

Edit:
I chose output_dim over dim_output to be consistent with the choice in models.iosys and also because I think it sounds nicer. However, this is somewhat inconsistent with the rest of pyMOR where 'dim_***' is used.

The dimension of the output is given by dim_output attribute. output_dim of IntputOutputModel has been renamed accordingly. Its input_dim attribute has been renamed to dim_input. The old names are deprecated.

In addition, input_space and output_space have been removed from InputOutputModel and the b and c arrays of tangential directions for the interpolatory reductors have been made numpy arrays instead of VectorArrays.

[sdrave]

@pmli, I'm not sure about the rom0 stuff. Probably should now be NumPy arrays instead of VectorArrays?

[pmli]

Removing output_space from iosys models feels like a big change and also like not enough of a big change. Using only output_dim seems like it implies that we always assume that the number of outputs is "small" and that the output space doesn't matter, which I'm not even sure how many implications it would have (e.g., it would change the assumptions methods like eval_tf need to make and using input/output space ids could be a way to implement a method for interconnecting systems). On the other hand, if it's necessary to remove it, it would make sense to also remove input_space. I'm not sure of all the advantages and disadvantages. One good thing is that adding assumptions that B and C can always be converted to a VectorArray would make eval_tf more "pyMOR-ic".

[sdrave]

Removing output_space from iosys models feels like a big change and also like not enough of a big change. Using only output_dim seems like it implies that we always assume that the number of outputs is "small" and that the output space doesn't matter, which I'm not even sure how many implications it would have (e.g., it would change the assumptions methods like eval_tf need to make and using input/output space ids could be a way to implement a method for interconnecting systems).

I'm really confused. Isn't this what we discussed the entire time? When outputs are always NumPy arrays, then what's the point of defining an output space?

On the other hand, if it's necessary to remove it, it would make sense to also remove input_space.

Of course. I havn't done this, as this PR is only about the outputs and we havn't formalized inputs yet. It clear however, that when outputs are NumPy arrays, than inputs as well. (But we still need to discuss time, Mu and so on ..)

[pmli]

I'm really confused. Isn't this what we discussed the entire time?

I thought we only discussed editing the solve and output methods...

When outputs are always NumPy arrays, then what's the point of defining an output space?

As I said, using space ids could be used to manage connecting inputs and outputs of different systems. In some situations, it would be natural to have two types of inputs/outputs, and then the input/output space could be a BlockVectorSpace where each subspace has a special id.

[sdrave]

I'm really confused. Isn't this what we discussed the entire time?

I thought we only discussed editing the solve and output methods...

output_space is part of output's and solve's api: it's the VectorSpace of the return values of these methods. If we change outputs to NumPy arrays but do not change output_space to output_dim you expose the user to a second notion of what the output seems to be. It is not clear to me, what the definition of this second notion actually would be and of what use it might be.

When outputs are always NumPy arrays, then what's the point of defining an output space?

As I said, using space ids could be used to manage connecting inputs and outputs of different systems. In some situations, it would be natural to have two types of inputs/outputs, and then the input/output space could be a BlockVectorSpace where each subspace has a special id.

If I don't misunderstand this, the same thing could be easily achieved with an output_ids or output_names attribute. I am open to add something like this.

[pmli]

I'm really confused. Isn't this what we discussed the entire time?

I thought we only discussed editing the solve and output methods...

output_space is part of output's and solve's api: it's the VectorSpace of the return values of these methods. If we change outputs to NumPy arrays but do not change output_space to output_dim you expose the user to a second notion of what the output seems to be. It is not clear to me, what the definition of this second notion actually would be and of what use it might be.

Ok, so, for you, C.range and D.range are something internal to LTIModel and output_space/output_dim are something separate? I'm wondering if we remove output_space and only use output_dim, should then C be a VectorArray/VectorArrayOperator?

When outputs are always NumPy arrays, then what's the point of defining an output space?

As I said, using space ids could be used to manage connecting inputs and outputs of different systems. In some situations, it would be natural to have two types of inputs/outputs, and then the input/output space could be a BlockVectorSpace where each subspace has a special id.

If I don't misunderstand this, the same thing could be easily achieved with an output_ids or output_names attribute. I am open to add something like this.

Ok, I guess that could work.

[sdrave]

output_space is part of output's and solve's api: it's the VectorSpace of the return values of these methods. If we change outputs to NumPy arrays but do not change output_space to output_dim you expose the user to a second notion of what the output seems to be. It is not clear to me, what the definition of this second notion actually would be and of what use it might be.

Ok, so, for you, C.range and D.range are something internal to LTIModel and output_space/output_dim are something separate?

Yes. I would go as far as saying that potentially having different C.range with the same C.range.dim is more of an annoyance than a feature if all outputs have to be NumPy arrays in the end. So one might even think about requiring that C.range always is a NumpyVectorSpace.

I'm wondering if we remove output_space and only use output_dim, should then C be a VectorArray/VectorArrayOperator?

I think I originally would have liked C and B to be VectorArrays, but we decided against that at some point. Don't remember all the details. One consequence would be that they no longer could be parametric. Also, it is probably not what people would expect.

[pmli]

Yes. I would go as far as saying that potentially having different C.range with the same C.range.dim is more of an annoyance than a feature if all outputs have to be NumPy arrays in the end. So one might even think about requiring that C.range always is a NumpyVectorSpace.

Yes, I agree requiring that C.range (and B.source) are NumpyVectorSpace would be nice, but I'm not sure it would work for interconnected systems as in

where system P has outputs z and v, and you would need to extract them somehow to assemble the interconnected system. I'm not sure that would be possible for a parametric C operator.

I think I originally would have liked C and B to be VectorArrays, but we decided against that at some point. Don't remember all the details. One consequence would be that they no longer could be parametric. Also, it is probably not what people would expect.

Yes, it would definitely be good if B and C could be parametric.

[sdrave]

Yes. I would go as far as saying that potentially having different C.range with the same C.range.dim is more of an annoyance than a feature if all outputs have to be NumPy arrays in the end. So one might even think about requiring that C.range always is a NumpyVectorSpace.

Yes, I agree requiring that C.range (and B.source) are NumpyVectorSpace would be nice, but I'm not sure it would work for interconnected systems as in

where system P has outputs z and v, and you would need to extract them somehow to assemble the interconnected system. I'm not sure that would be possible for a parametric C operator.

I am not quite sure how this is related to allowing output_space to be different from NumpyVectorSpace. If you are thinking of blocked spaces and assigning ids to the subspaces, as I said I am fine with assigning labels to every output component. That should give you enough information on how to connect things. If you want to interconnect Models to make a new Model, it should be trivial to write a new Operator that extracts a few components from a NumpyVectorArray and makes a new array from them.

I think I originally would have liked C and B to be VectorArrays, but we decided against that at some point. Don't remember all the details. One consequence would be that they no longer could be parametric. Also, it is probably not what people would expect.

Yes, it would definitely be good if B and C could be parametric.

Ok, so B and C should stay Operators.

[pmli]

If you want to interconnect Models to make a new Model, it should be trivial to write a new Operator that extracts a few components from a NumpyVectorArray and makes a new array from them.

Yes, for a NumpyMatrixOperator, it should be easy to extract a few rows/columns, but I'm not sure if this is possible for a general parametric (not in parameter-separable form) Operator. Although, I guess this could be achieved by concatenating with a ComponentProjectionOperator (it might not be the most efficient thing, but it should work).

[sdrave]

If you want to interconnect Models to make a new Model, it should be trivial to write a new Operator that extracts a few components from a NumpyVectorArray and makes a new array from them.

Yes, for a NumpyMatrixOperator, it should be easy to extract a few rows/columns, but I'm not sure if this is possible for a general parametric (not in parameter-separable form) Operator. Although, I guess this could be achieved by concatenating with a ComponentProjectionOperator (it might not be the most efficient thing, but it should work).

This is what I had in mind, actually. There might be a more convenient version where you can specify the names of the output components or so. Not sure about your concerns regarding efficiency. But there is also the option of having some version of BlockOperator which outputs stacked NumpyVectorArrays in case you want to dissect the operator itself.

[sdrave]

I have now updated this PR to current master and added the following changes:

• As I agree that having output_dim but input_space is really strange, I also replaced input_space by input_dim for the sys-mor models.
• While I was at it, it seemed to me unnatural to have the tangential directions b and c in the interpolatory reductors as VectorArrays, so I changed them to numpy arrays.

Before merging this: Who prefers dim_output over output_dim, @pymor/all? Note that elsewhere we have dim_source, dim_range, dim_domain, so dim_output might actually make more sense.

[codecov]

Codecov Report

Merging #1089 (7ee8c8e) into master (c602a0a) will increase coverage by 0.01%.
The diff coverage is 51.04%.

Impacted Files	Coverage Δ
src/pymor/models/neural_network.py	94.33% <0.00%> (ø)
src/pymor/reductors/bt.py	66.66% <ø> (ø)
src/pymor/reductors/sor_irka.py	97.43% <ø> (ø)
src/pymor/reductors/h2.py	54.51% <25.00%> (+0.15%)	⬆️
src/pymor/models/interface.py	66.29% <40.00%> (+1.07%)	⬆️
src/pymor/models/iosys.py	49.71% <44.18%> (-0.07%)	⬇️
src/pymor/reductors/interpolation.py	82.25% <81.25%> (+0.09%)	⬆️
src/pymor/models/basic.py	89.23% <100.00%> (+0.34%)	⬆️
src/pymordemos/delay.py	100.00% <100.00%> (ø)
src/pymordemos/heat.py	95.94% <100.00%> (ø)
... and 5 more

[ftalbrecht]

Before merging this: Who prefers dim_output over output_dim, @pymor/all? Note that elsewhere we have dim_source, dim_range, dim_domain, so dim_output might actually make more sense.

Agreed

[sdrave]

I just renamed input_dim/output_dim to dim_input/dim_output.

[sdrave]

@pmli, this can now be merged, IMHO. Can you take a look?

[sdrave]

@renefritze, there are some strange new build failures here ..

[renefritze]

@renefritze, there are some strange new build failures here ..

Restart or rebase on #1158