Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement mass-metallicity relations #58

Merged
merged 82 commits into from
Jan 25, 2025
Merged

Implement mass-metallicity relations #58

merged 82 commits into from
Jan 25, 2025

Conversation

cgarling
Copy link
Owner

@cgarling cgarling commented Jan 13, 2025

This PR implements mass-metallicity relations (MZRs) as an alternative to previously implemented age-metallicity relations (AMRs). In order to simplify implementation of these models, the hierarchical fitting API was overhauled so that fit_sfh is now the recommended entry point for hierarchical fitting and sample_sfh is the entry point for HMC sampling for hierarchical fitting. calculate_coeffs replaces calculate_coeffs_mdf.

Most of the old functions that are superceded by this new API still exist in this PR, but should issue deprecation warnings. These should eventually be removed in the 1.0 release.

Also fixes #51 and #52.

This commit implements most of the functionality developed in the test notebook. Tomorrow need to write tests for `fg_mzr!` and start on fitting function.
This should be moved into the model interface when we update the AMR models.
Want to support unsorted `logAge` arguments in `fg_mzr!`, will require some alterations.
Also adds `logdensity_and_gradient` definition that takes care of variable transformations, with optional Jacobian corrections.
`fittable_params` to retrieve the values of the fittable parameters out of models.
Not sure I want these to live here forever but can always move them later.
Implements random sampling and some other StatsBase methods
Contains MLE and MAP result from BFGS optimization. Also hoist transformation out of `_rand!` calls and into `exptransform_samples!` in hierarchical_models.jl to enable reuse of this code for HMC samples.
Warmup steps were fairly unsuccessful, so just check convergence after running and warn if poor.
Moved docs on hierarchical models from fitting/ to fitting/hierarchical to reflect structure in source code directory. Added `try/catch` block in `fit_sfh` to deal with inverse Hessian approximations in MLE results that are not positive definite. Added `calculate_coeffs` method to return types `BFGSResult` and `CompositeBFGSResult` with tests. Added docstring for `fit_sfh` in generic `fitting/hierarchical/hierarchical_models.jl` where it will probably be moved eventually.
Reduces runtime by 25%
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 84.13223% with 96 lines in your changes missing coverage. Please review.

Project coverage is 70.12%. Comparing base (42565c6) to head (03dc931).

Files with missing lines Patch % Lines
src/fitting/utilities.jl 6.45% 29 Missing ⚠️
src/fitting/hierarchical/generic_fitting.jl 89.54% 23 Missing ⚠️
src/utilities.jl 40.62% 19 Missing ⚠️
src/fitting/hierarchical/amr.jl 91.86% 10 Missing ⚠️
src/fitting/hierarchical/transformations.jl 78.04% 9 Missing ⚠️
src/fitting/hierarchical/bfgs_result.jl 90.00% 5 Missing ⚠️
src/fitting/hierarchical/linear_amr/linear_amr.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #58       +/-   ##
===========================================
- Coverage   88.60%   70.12%   -18.49%     
===========================================
  Files          18       25        +7     
  Lines        1536     2122      +586     
===========================================
+ Hits         1361     1488      +127     
- Misses        175      634      +459     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Still triggers `Test.detect_unbound_args` because of the `Vararg` but don't care right now.
New `tups_to_mat` function results in unbound type parameters because of `Vararg`, trying to fix. See [Aqua.jl #316](JuliaTesting/Aqua.jl#316) and [#86](JuliaTesting/Aqua.jl#86).
Seems to be passing Aqua checks now.
All tests passing, including Aqua
Update SFH fitting section
Having `N` in the method signature forces specialization, which recompiles for every different `N`, blows up compilation time for large `N` but is fine when using repeatedly with same, small `N`. Going to try removing specialization; tests indicate it does not really hurt performance.
@cgarling cgarling merged commit bbef228 into main Jan 25, 2025
8 checks passed
@cgarling cgarling deleted the MZR branch January 25, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to fix any fitting variables
2 participants