From 343c4e8bc30bb75b37d7006bd7ee6b3af8cd5a74 Mon Sep 17 00:00:00 2001 From: Bret Naylor Date: Fri, 26 May 2023 14:21:16 -0400 Subject: [PATCH 1/7] added poem 87 --- POEM_087.md | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 POEM_087.md diff --git a/POEM_087.md b/POEM_087.md new file mode 100644 index 00000000..a59ed210 --- /dev/null +++ b/POEM_087.md @@ -0,0 +1,83 @@ +POEM ID: 087 +Title: Expand functionality of dynamic shaping. +authors: @naylor-b +Competing POEMs: +Related POEMs: +Associated implementation PR: + +Status: + +- [x] Active +- [ ] Requesting decision +- [ ] Accepted +- [ ] Rejected +- [ ] Integrated + + +## Motivation + +Currently, the shape of a dynamically shaped variable can be determined either by the shape of +the variable it connects to, via 'shape_by_conn', or by the shape of a variable residing in the +same component, via 'copy_shape'. Both cases only work if the desired shape happens to be the +same as the shape of the connected or named variable. + +In the case of 'copy_shape' there are situations where the desired shape of an output will not +be the same as the shape of a single input but will instead be some function of the shapes of +one or more of the component's input variables. + + +## Proposed solutions + +Altering the allowed values of `copy_shape` to include a tuple of the form +`(var_names, func)`, where: + +- `var_names` is a list of variable names +- func is a function taking a dict arg of the form `{'var1': shape1, 'var2': shape2, ...}` +and returning the desired shape + + +would allow the desired shape to be computed based on the shapes of multiple variables from +the same component if necessary. + + +In the example below, the value of `copy_shape` is modified as to be a tuple as shown above, but +this brings up another issue. This new operation is not simply copying the shape of a specified +variable, so the name `copy_shape` isn't really accurate any more. There are three ways to address +this. + +1) We just ignore it and allow `copy_shape` to have either its typical string value or the new tuple +value +2) We rename `copy_shape` to something else like `compute_shape` and deprecate `copy_shape`. +3) We leave `copy_shape` as is and add a new metadata attribute called `compute_shape` that is +required to have a tuple value as explained above. + + +## Example + +A component has two dynamically shaped input matrices, 'M1' and 'M2', and an output matrix 'M3' +that is the result of the matrix multiplication M1 * M2. If 'M1' is shape (m, n) and 'M2' is +shape (n, p), then the desired shape of 'M3' is (m, p). This can be computed using the function + +``` +def shapefunc(shapes): + return (shapes['M1'][0], shapes['M2'][1]) +``` + +or + +``` +lambda shapes: (shapes['M1'][0], shapes['M2'][1]) +``` + + +So, when adding output 'M3' to its parent component, the add_output call would look something +like this: + +``` +self.add_output('M3', copy_shape=(['M1', 'M2'], shapefunc)) +``` + + +``` +self.add_output('M3', copy_shape=(['M1', 'M2'], lambda shapes: (shapes['M1'][0], shapes['M2'][1]))) +``` \ No newline at end of file From 2f7fb68ccd6ea33d5c134162aa94723feaed4bb4 Mon Sep 17 00:00:00 2001 From: Bret Naylor Date: Fri, 26 May 2023 14:24:49 -0400 Subject: [PATCH 2/7] small fix --- POEM_087.md | 1 + 1 file changed, 1 insertion(+) diff --git a/POEM_087.md b/POEM_087.md index a59ed210..8f53be72 100644 --- a/POEM_087.md +++ b/POEM_087.md @@ -77,6 +77,7 @@ like this: self.add_output('M3', copy_shape=(['M1', 'M2'], shapefunc)) ``` +or ``` self.add_output('M3', copy_shape=(['M1', 'M2'], lambda shapes: (shapes['M1'][0], shapes['M2'][1]))) From 2cbea083fd3e730a0d5374f43cae46e6e7aea407 Mon Sep 17 00:00:00 2001 From: Bret Naylor Date: Thu, 20 Jul 2023 13:23:59 -0400 Subject: [PATCH 3/7] updated POEM --- POEM_087.md | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/POEM_087.md b/POEM_087.md index 8f53be72..451f99ad 100644 --- a/POEM_087.md +++ b/POEM_087.md @@ -26,30 +26,14 @@ be the same as the shape of a single input but will instead be some function of one or more of the component's input variables. -## Proposed solutions +## Proposed solution -Altering the allowed values of `copy_shape` to include a tuple of the form -`(var_names, func)`, where: - -- `var_names` is a list of variable names -- func is a function taking a dict arg of the form `{'var1': shape1, 'var2': shape2, ...}` -and returning the desired shape - - -would allow the desired shape to be computed based on the shapes of multiple variables from -the same component if necessary. - - -In the example below, the value of `copy_shape` is modified as to be a tuple as shown above, but -this brings up another issue. This new operation is not simply copying the shape of a specified -variable, so the name `copy_shape` isn't really accurate any more. There are three ways to address -this. - -1) We just ignore it and allow `copy_shape` to have either its typical string value or the new tuple -value -2) We rename `copy_shape` to something else like `compute_shape` and deprecate `copy_shape`. -3) We leave `copy_shape` as is and add a new metadata attribute called `compute_shape` that is -required to have a tuple value as explained above. +Since `copy_shape` doesn't properly describe the process of computing the shape based on the +shapes of other variables, a new argument called `compute_shape` will be added to `add_output` and +`add_input`. The value of `compute_shape` will be a function taking a single argument of the form +`{'var1': shape1, 'var2': shape2, ...}`. The argument will be populated with shapes of +all input variables to the component. This will allow the final shape to be computed based on the +shapes of multiple variables if necessary. ## Example @@ -74,11 +58,11 @@ So, when adding output 'M3' to its parent component, the add_output call would l like this: ``` -self.add_output('M3', copy_shape=(['M1', 'M2'], shapefunc)) +self.add_output('M3', compute_shape=(shapefunc)) ``` or ``` -self.add_output('M3', copy_shape=(['M1', 'M2'], lambda shapes: (shapes['M1'][0], shapes['M2'][1]))) +self.add_output('M3', compute_shape=(lambda shapes: (shapes['M1'][0], shapes['M2'][1]))) ``` \ No newline at end of file From f4fae018b37ccb9556e67b2b1ac3ca3e055d8c02 Mon Sep 17 00:00:00 2001 From: Bret Naylor Date: Tue, 15 Aug 2023 13:48:22 -0400 Subject: [PATCH 4/7] removed unnecessary parens --- POEM_087.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/POEM_087.md b/POEM_087.md index 451f99ad..4267b857 100644 --- a/POEM_087.md +++ b/POEM_087.md @@ -58,11 +58,11 @@ So, when adding output 'M3' to its parent component, the add_output call would l like this: ``` -self.add_output('M3', compute_shape=(shapefunc)) +self.add_output('M3', compute_shape=shapefunc) ``` or ``` -self.add_output('M3', compute_shape=(lambda shapes: (shapes['M1'][0], shapes['M2'][1]))) -``` \ No newline at end of file +self.add_output('M3', compute_shape=lambda shapes: (shapes['M1'][0], shapes['M2'][1])) +``` From 4d50981a6d91c2db8deb6fc2c0ddf6ffea6ef38e Mon Sep 17 00:00:00 2001 From: Bret Naylor Date: Wed, 23 Aug 2023 13:33:48 -0400 Subject: [PATCH 5/7] updated impl PR --- POEM_087.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/POEM_087.md b/POEM_087.md index 4267b857..5b4d156b 100644 --- a/POEM_087.md +++ b/POEM_087.md @@ -3,7 +3,7 @@ Title: Expand functionality of dynamic shaping. authors: @naylor-b Competing POEMs: Related POEMs: -Associated implementation PR: +Associated implementation PR: [OpenMDAO/OpenMDAO#3000 : Implementation branch for POEM 87 Expand Functionality of Dynamic Shaping](https://github.com/OpenMDAO/OpenMDAO/pull/3000) Status: @@ -32,14 +32,14 @@ Since `copy_shape` doesn't properly describe the process of computing the shape shapes of other variables, a new argument called `compute_shape` will be added to `add_output` and `add_input`. The value of `compute_shape` will be a function taking a single argument of the form `{'var1': shape1, 'var2': shape2, ...}`. The argument will be populated with shapes of -all input variables to the component. This will allow the final shape to be computed based on the +all input variables in the component. This will allow the final shape to be computed based on the shapes of multiple variables if necessary. ## Example A component has two dynamically shaped input matrices, 'M1' and 'M2', and an output matrix 'M3' -that is the result of the matrix multiplication M1 * M2. If 'M1' is shape (m, n) and 'M2' is +that is the result of the matrix multiplication M1 @ M2. If 'M1' is shape (m, n) and 'M2' is shape (n, p), then the desired shape of 'M3' is (m, p). This can be computed using the function ``` From f861259703d13bbb38009511664ef8272858c989 Mon Sep 17 00:00:00 2001 From: Rob Falck Date: Thu, 24 Aug 2023 11:41:04 -0400 Subject: [PATCH 6/7] Update POEM_087.md Accepted POEM 087 --- POEM_087.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/POEM_087.md b/POEM_087.md index 5b4d156b..bad5c18b 100644 --- a/POEM_087.md +++ b/POEM_087.md @@ -7,9 +7,9 @@ Associated implementation PR: [OpenMDAO/OpenMDAO#3000 : Implementation branch fo Status: -- [x] Active +- [ ] Active - [ ] Requesting decision -- [ ] Accepted +- [x] Accepted - [ ] Rejected - [ ] Integrated From ddb3bab31116d9d75c8693e77ad6ea7cf9fae317 Mon Sep 17 00:00:00 2001 From: Rob Falck Date: Thu, 24 Aug 2023 11:42:29 -0400 Subject: [PATCH 7/7] Update POEM_087.md Formatting --- POEM_087.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/POEM_087.md b/POEM_087.md index bad5c18b..b98483b7 100644 --- a/POEM_087.md +++ b/POEM_087.md @@ -1,6 +1,6 @@ POEM ID: 087 Title: Expand functionality of dynamic shaping. -authors: @naylor-b +authors: naylor-b (Bret Naylor) Competing POEMs: Related POEMs: Associated implementation PR: [OpenMDAO/OpenMDAO#3000 : Implementation branch for POEM 87 Expand Functionality of Dynamic Shaping](https://github.com/OpenMDAO/OpenMDAO/pull/3000)