Adaptive MCMC in the refactor! #79

henry-wallace-phys · 2024-07-18T15:44:04Z

Pull request description:

Ports adaptive MCMC to the MaCh3 refactor.

Changes or fixes:

Easier to use interface, now takes in a YAML node of the form

AdaptionOptions:
  Settings:
    # When do we start throwing from our adaptive matrix?
    AdaptionStartThrow : 2000
    # When do we start putting steps into our adaptive covariance?
    AdaptionStartUpdate : 0
    # When do we end updating our covariance?
    AdaptionEndUpdate : 50000
    # How often do we change our matrix throws?
    AdaptionUpdateStep : 1000
  Covariance:
    # So now we list individual matrices, let's just do xsec
    xsec_cov:
      # Do we want to adapt this matrix?
      DoAdaption: True
      # So we can separate out flux and xsec
      MatrixBlocks: [ [45, 145] ]
      # External Settings
      UseExternalMatrix: false
      ExternalMatrixFileName: ""
      ExternalMatrixName: ""
      ExternalMeansName: ""

Updated adaptive MCMC calculation to use a block matrix approach.
Code is slightly more readable [...hopefully]

Examples:

Fit performed on JUST parameter priors

test_t2k_outputfile_not_delayed_fixdcp_python.pdf_posteriors.pdf

…into feature_adaption_update

KSkwarczynski

Thanks a lot for this

KSkwarczynski · 2024-07-18T21:43:11Z

covariance/covarianceBase.cpp

+// HW : Here be adaption
+void covarianceBase::initialiseAdaption(manager* fitMan){
+  /*
+    HW: Idea is that adaption can simply read the YAML config
+    Options :


Can you pass const YAML::Node& Adapt isntead of fitMan.
Seem cleaner

KSkwarczynski · 2024-07-18T21:44:36Z

covariance/covarianceBase.cpp

+  }
+
+   // Finally, we accept that we want to read the matrix from a file!
+
+  setThrowMatrixFromFile(fitMan->raw()["AdaptionOptions"]["Covariance"][matrixName_str]["ExternalMatrixFileName"].as<std::string>(),
+                         fitMan->raw()["AdaptionOptions"]["Covariance"][matrixName_str]["ExternalMatrixName"].as<std::string>(),
+                         fitMan->raw()["AdaptionOptions"]["Covariance"][matrixName_str]["ExternalMeansName"].as<std::string>());


Can you add some prinout wiht basic info about adaptive settings? Additional Verbose is always helpfull

KSkwarczynski · 2024-07-18T21:45:28Z

covariance/covarianceBase.cpp

+  start_adaptive_throw  = fitMan->raw()["AdaptionOptions"]["Settings"]["AdaptionStartThrow"].as<int>();
+  start_adaptive_update =  fitMan->raw()["AdaptionOptions"]["Settings"]["AdaptionStartUpdate"].as<int>();
+  end_adaptive_update   =  fitMan->raw()["AdaptionOptions"]["Settings"]["AdaptionEndUpdate"].as<int>();
+  adaptive_update_step  =  fitMan->raw()["AdaptionOptions"]["Settings"]["AdaptionUpdateStep"].as<int>();


Can you use GetFromManager function to set default values and avoid crushes due to bad yaml?

KSkwarczynski · 2024-07-18T21:46:46Z

covariance/covarianceBase.h

+  // Indices for block-matrix adaption
+  std::vector<int> adapt_block_matrix_indices;
+  // Size of blocks for adaption
+  std::vector<int> adapt_block_sizes;


nitpicky but if you replace // with /// above variable then this is recongised by Doxygen. Would be nice to have this in doxy docueamtnion

KSkwarczynski · 2024-07-18T21:48:52Z

covariance/covarianceBase.cpp

+  adaptiveCovariance = new TMatrixDSym(size);
+  adaptiveCovariance->Zero();
+  par_means = std::vector<double>(size, 0);


I think I just noticed this but adaptiveCovariance is not being destroyed in destructor

KSkwarczynski · 2024-07-18T21:50:24Z

covariance/covarianceBase.cpp

+      if(block_lb>getNpars() || block_ub>getNpars()){
+	MACH3LOG_ERROR("Cannot set matrix block with edges {}, {} for matrix of size {}",
+		       block_lb, block_ub, getNpars());
+	throw;


Can you replace throw with
throw MaCh3Exception(__FILE__ , __LINE__ );

KSkwarczynski · 2024-07-18T21:52:17Z

covariance/covarianceBase.cpp

+  outFile->cd();
+  adaptiveCovariance->Write(systematicName+"_postfit_matrix");
+  outMeanVec->Write(systematicName+"_mean_vec");
+  outFile->Close();


I would delete outMeanVec just in case. i don't trust ROOT

KSkwarczynski · 2024-07-18T21:52:48Z

covariance/covarianceBase.cpp

+void covarianceBase::setThrowMatrixFromFile(std::string matrix_file_name, std::string matrix_name, std::string means_name){
+  // Lets you set the throw matrix externally
+  // Open file
+  std::unique_ptr<TFile>matrix_file(new TFile(matrix_file_name.c_str()));


OMG more smart poiinters

SMART POINTERS 4 LIFE

KSkwarczynski · 2024-07-18T21:56:05Z

covariance/covarianceBase.h

+  // When do we start throwing?
+  int start_adaptive_throw;
+  //Thresholds for when to turn on/off updating adaptive MCMC
+  int start_adaptive_update;
+  // When do we stop update the adaptive matrix?
+  int end_adaptive_update;
+  // Steps between changing throw matrix
+  int adaptive_update_step;


I wonder wheter it make sesne to make small adaptation struct. Might be cleaner way to separate adaptive way.
Also if most of adaptive things are in struct then it is easier to move stuff from cov to Actual MCMC algorithm. Not that it will happen soon.

Up to you

…akes YAML node

Henry Wallace and others added 9 commits June 6, 2024 11:47

safety commit, updates interfaces etc.

f76224f

Adds updated adaptive MCMC to refactor

ef3e754

reserve merge conflicts to work from Kamils CUDA update branch

0db7333

Removes methods that accidentially got re-added during merge

6c8ef86

Updates YAML names

ff5aa9c

Merge branch 'feature_CudaUpdate' of github.com:mach3-software/MaCh3 …

536b7f8

…into feature_adaption_update

updates to covriance base

92aee34

adds check to make sure block sizes are correct (this w....

5e98e27

fix merge conflict, add comments

754f169

henry-wallace-phys requested a review from KSkwarczynski July 18, 2024 15:44

KSkwarczynski approved these changes Jul 18, 2024

View reviewed changes

henry-wallace-phys added 2 commits July 19, 2024 06:17

Few small updates, adaption stuff now moved to a struct, setter now t…

90d1a11

…akes YAML node

doxygen fixes

6e5dfee

KSkwarczynski merged commit f44d96d into develop Jul 19, 2024
3 checks passed

KSkwarczynski deleted the feature_adaption_update branch July 19, 2024 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adaptive MCMC in the refactor! #79

Adaptive MCMC in the refactor! #79

henry-wallace-phys commented Jul 18, 2024

KSkwarczynski left a comment

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

KSkwarczynski Jul 18, 2024

henry-wallace-phys Jul 19, 2024

KSkwarczynski Jul 18, 2024

Adaptive MCMC in the refactor! #79

Adaptive MCMC in the refactor! #79

Conversation

henry-wallace-phys commented Jul 18, 2024

Pull request description:

Changes or fixes:

Examples:

KSkwarczynski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment