Skip to content

899 new add gamma distribution and other parameters to state age function 1#1004

Merged
annawendler merged 39 commits intomainfrom
899-new-add-gamma-distribution-and-other-parameters-to-state_age_function-1
May 13, 2024
Merged

899 new add gamma distribution and other parameters to state age function 1#1004
annawendler merged 39 commits intomainfrom
899-new-add-gamma-distribution-and-other-parameters-to-state_age_function-1

Conversation

@lenaploetzke
Copy link
Member

@lenaploetzke lenaploetzke commented Apr 11, 2024 •
edited by annawendler
Loading

Changes and Information

Please briefly list the changes (main added features, changed items, or corrected bugs) made:

  • added the parameters location and scale to StateAgeFunction
  • added three state age functions
  • added tests

If need be, add additional information and what the reviewer should look out for in particular:

Merge Request - Guideline Checklist

Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.

Checks by code author

  • Every addressed issue is linked (use the "Closes #ISSUE" keyword below)
  • New code adheres to coding guidelines
  • No large data files have been added (files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
  • Tests are added for new functionality and a local test run was successful (with and without OpenMP)
  • Appropriate documentation for new functionality has been added (Doxygen in the code and Markdown files if necessary)
  • Proper attention to licenses, especially no new third-party software with conflicting license has been added
  • (For ABM development) Checked benchmark results and ran and posted a local test above from before and after development to ensure performance is monitored.

Checks by code reviewer(s)

  • Corresponding issue(s) is/are linked and addressed
  • Code is clean of development artifacts (no deactivated or commented code lines, no debugging printouts, etc.)
  • Appropriate unit tests have been added, CI passes, code coverage and performance is acceptable (did not decrease)
  • No large data files added in the whole history of commits(files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
  • On merge, add 2-5 lines with the changes (main added features, changed items, or corrected bugs) to the merge-commit-message. This can be taken from the briefly-list-the-changes above (best case) or the separate commit messages (worst case).

Closes #929.

lenaploetzke and others added 30 commits April 3, 2024 11:05
@lenaploetzke
added first ideas
5075322
@lenaploetzke
added command for b2 headers
ac16bc6
@lenaploetzke
change command for msvc
13e4d4a
@lenaploetzke
changed to .bat
6820e42
@lenaploetzke
delete invalid character
cb02f40
@lenaploetzke
use other slashes
f8e7be6
@lenaploetzke
try "
da1a8c0
@lenaploetzke
rm ./
eb59ff2
@lenaploetzke
update boost version
3ff1487
@lenaploetzke
rm auto keyword
84f3911
@lenaploetzke
add auto
23b08cf
@lenaploetzke
try what happens
f94fa5d
@lenaploetzke
add .exe
7ce0e7a
@lenaploetzke
try two backslashes
7d53977
@lenaploetzke
add cxx standard
262496f
@lenaploetzke
rm quiet flag
12bc2f4
@lenaploetzke
add quiet flag
556b89a
@lenaploetzke
added flag
0c12918
@lenaploetzke
add start
e930a90
@lenaploetzke
add help flag
cd0c24b
@lenaploetzke
try dir
7e84851
@lenaploetzke
just b2
ebbe4e1
@lenaploetzke
problem with "
19a0f9f
@lenaploetzke
b2 quiet again
9bab72c
@lenaploetzke
adapted documentation
f84d083
@lenaploetzke
added if clause
653334c
@lenaploetzke @reneSchm
Included reviewer suggestions
6e09cc6 
Co-authored-by: reneSchm <49305466+reneSchm@users.noreply.github.com>
@lenaploetzke @reneSchm
Update cpp/memilio/math/README.md
7517f99 
Co-authored-by: reneSchm <49305466+reneSchm@users.noreply.github.com>
@lenaploetzke
Merge branch '983-bundle-the-boost-git-repo-instead-of-using-the-targ…
7711fd1 
…z-archiv' into 899-new-add-gamma-distribution-and-other-parameters-to-state_age_function-1
@lenaploetzke
readd changes in state age function
250d102
@lenaploetzke lenaploetzke mentioned this pull request Apr 12, 2024
Closed
12 tasks
@lenaploetzke lenaploetzke added the status::in review This issues is in review. label Apr 12, 2024
@lenaploetzke lenaploetzke requested review from annawendler and removed request for mknaranja April 23, 2024 12:21
@annawendler
Copy link
Member

annawendler commented Apr 29, 2024

Can you merge the main branch so that the get_mean method is already included here?

@lenaploetzke lenaploetzke removed the request for review from annawendler April 29, 2024 13:55
@lenaploetzke lenaploetzke added status::on hold This issue is on hold. status::in progress This issue is in progess. and removed status::in review This issues is in review. status::on hold This issue is on hold. labels Apr 29, 2024
@lenaploetzke lenaploetzke added status::in review This issues is in review. and removed status::in progress This issue is in progess. labels Apr 30, 2024
annawendler
annawendler requested changes May 10, 2024
View reviewed changes
Copy link
Member

@annawendler annawendler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

I was wondering if we should rename ExponentialDecay to ExponentialSurvivalFunction. This would be consistent with the other new SAFs and describe better what it is used for. I'm not sure if SmootherCosineSurvivalFunction is a good name as I don't know if there is a name for the associated distribution, but I guess we should at least mention that this is a survival function as well in the documentation.

@lenaploetzke @annawendler
Apply suggestions from code review
ab509b0 
Co-authored-by: annawendler <106674756+annawendler@users.noreply.github.com>
@lenaploetzke
Copy link
Member Author

lenaploetzke commented May 10, 2024

Nice work!

I was wondering if we should rename ExponentialDecay to ExponentialSurvivalFunction. This would be consistent with the other new SAFs and describe better what it is used for. I'm not sure if SmootherCosineSurvivalFunction is a good name as I don't know if there is a name for the associated distribution, but I guess we should at least mention that this is a survival function as well in the documentation.

I like this idea and renamed ExponentialDecay. I also agree, that the name SmootherCosineSurvivalFunction is not appropriate, but put in the docu.

@lenaploetzke lenaploetzke requested a review from annawendler May 13, 2024 08:43
annawendler
annawendler requested changes May 13, 2024
View reviewed changes
lenaploetzke and others added 2 commits May 13, 2024 14:45
@lenaploetzke @annawendler
Apply suggestions from code review
8cf9f4c 
Co-authored-by: annawendler <106674756+annawendler@users.noreply.github.com>
@lenaploetzke
changed smoothercosine to lognorm in test
92d7a37
@lenaploetzke lenaploetzke requested a review from annawendler May 13, 2024 14:05
@annawendler annawendler merged commit e15b4bb into main May 13, 2024
@annawendler annawendler deleted the 899-new-add-gamma-distribution-and-other-parameters-to-state_age_function-1 branch May 13, 2024 15:16
reneSchm pushed a commit that referenced this pull request May 24, 2024
@lenaploetzke @annawendler @reneSchm
899 new add gamma distribution and other parameters to state age func…
6bc9452 
…tion (#1004)

- Added the parameters location and scale to StateAgeFunction
- Added three state age functions: GammaSurvivalFunction, LognormSurvivalFunction and ErlangDensity
- Renamed ExponentialDecay to ExponentialSurvivalFunction and the member m_parameter to m_distribution_parameter
- Added tests

Co-authored-by: annawendler <106674756+annawendler@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@annawendler annawendler annawendler approved these changes

Assignees

@lenaploetzke lenaploetzke

Labels

status::in review This issues is in review.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add gamma distribution and other parameters to state_age_function

2 participants

@lenaploetzke @annawendler