Skip to content
Snippets Groups Projects

Fix logic in produce_fitthcovmat

Merged Fix logic in produce_fitthcovmat
configthcov into master
Merged Emanuele Roberto Nocera requested to merge configthcov into master

Created by: wilsonmr

closes #433 (closed)

Before one couldn't set use_theorycovmat to False. This removes the option to set as string since that was messy and improves the logic so that one cat set use_theorycovmat and if you do then you don't need to provide a fit either

Merge request reports

Merged by avatar (Apr 5, 2025 11:51am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Emanuele Roberto Nocera
    Emanuele Roberto Nocera @enocera started a thread on commit 364929bb
    • validphys2/src/validphys/config.py
    709 raise ConfigError(
    710 f"No file found at {use_theorycovmat}. If specifying "
    711 "`use_theorycovmat: <path to covmat>` then <path to covmat> should point at a "
    712 "valid theory covariance matrix.")
    713 elif isinstance(use_theorycovmat, bool) and use_theorycovmat:
    702
    703 if not use_theorycovmat:
    704 pass
    705 elif not fit:
    706 log.warning("use_theorycovmat was true but no fit was specified, theory covariance "
    707 "matrix will not be used in any statstical estimators")
    708 use_theorycovmat = False
    709 elif use_theorycovmat and fit:
    714 710 try:
    715 711 use_theorycovmat = fit.as_input()['theorycovmatconfig']['use_thcovmat_in_fitting']
    716 712 except KeyError:
  • Emanuele Roberto Nocera
  • Emanuele Roberto Nocera
    Emanuele Roberto Nocera @enocera ·
    Author Owner

    Created by: Zaharid

    Actually I am not sure I like that you can set the flag and not have a fit with the covmat. Maybe if we called it something differently, like use_thcovmat_if_present. I don't know.

  • Emanuele Roberto Nocera
    Emanuele Roberto Nocera @enocera ·
    Author Owner

    Created by: wilsonmr

    sure, I guess the flag is really whether or not to use the covariance matrix as used in fit, which most of the time I guess we do, I'm not really bothered about the name but it is a bit inaccurate I guess. I think in the future (possibly after #435 (closed)) the theorycovmat should have something similar to the cuts: internal (calculated by vp) and then fromfit/None (what this function is doing)

  • Emanuele Roberto Nocera
    Emanuele Roberto Nocera @enocera ·
    Author Owner

    Created by: wilsonmr

    I don't think I can simplify the logic further and still have this do everything we want, hopefully this reads a bit better though

  • Emanuele Roberto Nocera
    Emanuele Roberto Nocera @enocera ·
    Author Owner

    Created by: Zaharid

    Review: Approved

  • Emanuele Roberto Nocera
    Emanuele Roberto Nocera @enocera ·
    Author Owner

    Merged by: Zaharid at 2019-04-17 11:07:24 UTC

  • Emanuele Roberto Nocera closed

    closed

  • Emanuele Roberto Nocera merged manually

    merged manually

  • Emanuele Roberto Nocera mentioned in issue #441 (closed)

    mentioned in issue #441 (closed)

    • Please register or sign in to reply