Skip to content

New QU-fitting models, and minor bug fix#72

Merged
AlecThomson merged 13 commits intoCIRADA-Tools:new_qufitfrom
jackieykma:master
Jul 5, 2023
Merged

New QU-fitting models, and minor bug fix#72
AlecThomson merged 13 commits intoCIRADA-Tools:new_qufitfrom
jackieykma:master

Conversation

@jackieykma
Copy link
Copy Markdown
Contributor

Hi Cameron,

I've made the following changes for the QU-fitting:

  • Added new models for QU-fitting
  • Improved the descriptions of the existing models (with relevant references) within each model .py files
  • Changed sigma_RM prior from 1000 radm-2 to 100 radm-2 (which I would argue that, for most cases, are more useful & realistic, and of course people applying QU-fitting to very high frequency data can simply change the prior range themselves as they see fit)
  • Fixed a bug in the error code in do_QUfit_1D_mnest.py --- compare with line 227 which attempts to load in the model files

Happy to discuss about any of these changes as needed!

Copy link
Copy Markdown
Contributor

@AlecThomson AlecThomson left a comment

Choose a reason for hiding this comment

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

Nice updates @jackieykma ! Whilst we're in here I think we can also change the polfrac minima to 0 in the priors. I think most fitter are robust enough to handle this

@jackieykma jackieykma requested a review from AlecThomson June 28, 2023 01:57
@jackieykma
Copy link
Copy Markdown
Contributor Author

Apologies for the delay! I've made the changes of minimum fracpol to 0.0 as requested

@AlecThomson
Copy link
Copy Markdown
Contributor

Hey @jackieykma - this is looking great! I may make one other change to use some simple for loops to avoid repetition in the multi-component files. But that can happen later!

One final request - could you please add some tests for the new models? See tests/QA_tests.py

@AlecThomson AlecThomson self-assigned this Jul 2, 2023
@AlecThomson AlecThomson added enhancement New feature or request WIP Work in-progress bug Something isn't working labels Jul 2, 2023
@AlecThomson AlecThomson self-requested a review July 2, 2023 03:11
@AlecThomson
Copy link
Copy Markdown
Contributor

P.S. if you want a hand to develop the tests, happy to help out

Copy link
Copy Markdown
Contributor

@AlecThomson AlecThomson left a comment

Choose a reason for hiding this comment

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

We need to add some tests...

@AlecThomson AlecThomson changed the base branch from master to new_qufit July 5, 2023 01:57
@AlecThomson AlecThomson merged commit 914b7d5 into CIRADA-Tools:new_qufit Jul 5, 2023
@AlecThomson
Copy link
Copy Markdown
Contributor

@jackieykma I've merged into a new branch and I'll take care of the tests :)

Cameron-Van-Eck added a commit that referenced this pull request Mar 20, 2024
* New QU-fitting models, and minor bug fix (#72)

* Improved model files to be more descriptive, and added 3 new models

* Fixed error message not showing correct path to model files

* Added triple Faraday thin model

* Fixed indentation error

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Changed fracpol minimum to 0.0

* Test models

* Standardise test names

* Ignore

* Testing tests

* Verbose output

* Updated new models to better LaTeX strings. Made math of models immune to black reformatting, for readability.

* Re-introduced Alec's new QUfitting tests. Disabled the values tests, because they were too sensitive to variations between runs. Only tests if QU-fitting runs, but not the output values.

* Fixing tests.

---------

Co-authored-by: Yik Ki (Jackie) Ma <yikki.ma@anu.edu.au>
Co-authored-by: Alec Thomson (S&A, Kensington WA) <alec.thomson@csiro.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request WIP Work in-progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants