Identity Weighting Matrix

optimagic-dev / optimagic

optimagic is a Python package for numerical optimization. It is a unified interface to optimizers from SciPy, NlOpt and other packages. optimagic's minimize function works just like SciPy's, so you don't have to adjust your code. You simply get more optimizers for free. On top you get diagnostic tools, parallel numerical derivatives and more.

https://optimagic.readthedocs.io/

MIT License

270 stars 30 forks source link

Identity Weighting Matrix #477

Closed sidd3888 closed 9 months ago

sidd3888 commented 10 months ago

Added the identity weighting matrix to the MSM estimation:

First change made is to the get_weighting_matrix() function to allow for the identity matrix
Second change permits "identity" as a valid option for weights in estimate_msm()

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (b77deb8) 92.98% compared to head (2fef810) 92.98%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #477 +/- ## ======================================= Coverage 92.98% 92.98% ======================================= Files 193 193 Lines 14631 14635 +4 ======================================= + Hits 13605 13609 +4 Misses 1026 1026 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

janosg commented 9 months ago

Hi @sidd3888, thank you very much for your Pull Request. The changes are great. There is just one more thing: It would be great to also have a small test case for the get_weighting_matrix with the "identity" option.

Let me know whether you want to add this test or I should do it. If you want to add the test, it should be in this file

sidd3888 commented 9 months ago

If I understand the code there correctly, this would require nothing more than just adding "identity" as a possible option (and accordingly altering the test) for the get_weighting_matrix test right? I can add that to this PR right away.

janosg commented 9 months ago

Yes. Thank you so much!