#3127 Add log sum exp func #3131
Conversation
…-log_sum_exp-func
…math into add-log_sum_exp-func
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
|
What about the row wise or column wise summation of matrices? Would that get a different function name? Just for my understanding: The intention of the function is to add two containers a and b using the log-sum-exp thing and returning again a container, right? |
Correct, that is what @spinkney requested at least |
|
Ok my apologies, I think i misinterpreted the original issue, I’ll work it on it tonight and incorporate the log_sum_exp into log_add_exp and ensure the tests reflect the change of only returning containers, should I assume the user is only allowed to pass in containers a and b, or should I account for different data types passed in. Thank you |
|
I think the only requirement should be that the containers passed must be the same size, but if one is a row vector and the other is a column vector I would expect that to be fine, I think? |
|
Should I account for multi-dimensional matrix containers or assume that only vectors or 1 dimensional matrix's are accepted into the method regardless if they are column or row-wise? |
|
I think the most useful thing would probably be to work over arbitrary containers of the same size. I think the apply_scalar_binary helpers may automate that |
Been trying to implement this, however, I'm facing an issue where if I'm given 2 different matrix types in which the dimensions don't align, I am not able to pass them into apply_scalar_binary, I have tried a few solutions by transposing the matrix and casting it to ensure its type doesn't change, yet that didn't work as it caused vectors to start failing as a result. Perhaps we should restrict matrix containers if they are not both the same size and have the same containers? Or is there a solution I can attempt, that I haven't tried yet? |
|
I may be misunderstanding, but I think if both arguments are matricies I think it is correct to require them to be the same size. We don’t generally do any “broadcasting” besides maybe allowing a scalar and a container |
|
Ok, thank you, in that case, I'll throw an exception if two matrix's passed and their dimensions are not aligned. |
|
I believe check_matching_dims does what we’d want it to do for that purpose |
|
Finished implementing the above changes, to clarify though, I understand this method will be used in the row-wise or column-wise summation of matrices. Will this be done outside of the method based on how the user utilizes it (passing in pairs of rows and columns)? At the moment, the code can add two containers a and b using the log-sum-exp method and returns a container of the same type as the parameters passed in. Any matrices that are passed in have element-wise operations performed on them before returning a container with the result of the same type. |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
|
Sorry for my delay in looking at this. Bit of a busy week but I will try to look on Friday. Glancing at everything I didn't see anything that was a huge blocker |
Summary
As discussed in issue #3127, I've implemented a log sum exp function that computes the logarithm of the element-wise sum of exponentials and returns the result as a container.
I've implemented 3 files + a test file in both prim, rev, and fwd, handling different edge cases that may arise.
Tests
The tests are written in the following file path, I modelled them after the tests for log_sum_exp as a base example:
test/unit/math/mix/fun/log_add_exp_test.cpp
Side Effects
None
Release notes
log_add_exp will be available if merged
Checklist
Copyright holder: (fill in copyright holder information)
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested