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

Feature tidy up #189

Merged
merged 12 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions python/samplePDF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
#include <pybind11/stl.h>
#include <pybind11/numpy.h>

#include "samplePDF/samplePDFBase.h"
#include "samplePDF/samplePDFFDBase.h"
#include "samplePDF/FDMCStruct.h"

namespace py = pybind11;

Expand Down
2 changes: 1 addition & 1 deletion samplePDF/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ set(HEADERS
samplePDFBase.h
samplePDFFDBase.h
Structs.h
FDMCStruct.h
FarDetectorCoreInfoStruct.h
Copy link
Contributor

Choose a reason for hiding this comment

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

I know its minor, but I'd remove struct from the file name, its very common to have a file named after the class it is defining. And having 'struct' in the name of a class/struct is just noise. (I also think 'Core' is noise here, and FarDetData or something is a cleaner name, but your name is absolutely fine and we don't need to argue forever about names)

)

add_library(SamplePDF SHARED
Expand Down
61 changes: 0 additions & 61 deletions samplePDF/FDMCStruct.h

This file was deleted.

68 changes: 68 additions & 0 deletions samplePDF/FarDetectorCoreInfoStruct.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#pragma once

/// @brief constructors are same for all three so put in here
struct FarDetectorCoreInfo {
FarDetectorCoreInfo() : isNC{nullptr} {}
FarDetectorCoreInfo(FarDetectorCoreInfo const &other) = delete;
FarDetectorCoreInfo(FarDetectorCoreInfo &&other) = default;
FarDetectorCoreInfo& operator=(FarDetectorCoreInfo const &other) = delete;
FarDetectorCoreInfo& operator=(FarDetectorCoreInfo &&other) = delete;

~FarDetectorCoreInfo(){ delete [] isNC; }

int nutype; // 2 = numu/signue | -2 = numub | 1 = nue | -1 = nueb
int oscnutype;
int nupdg;
int nupdgUnosc;
bool signal; // true if signue
int nEvents; // how many MC events are there
double ChannelIndex;
std::string flavourName;

std::vector<int*> Target; // target the interaction was on

int SampleDetID;

//THe x_var and y_vars that you're binning in
std::vector<const double*> x_var;
std::vector<const double*> y_var;
std::vector<const double*> rw_etru;
std::vector<const double*> rw_truecz;

/// xsec bins
std::vector< std::vector< int > > xsec_norms_bins;

/// DB Speedup bits
double Unity;
float Unity_F;
int Unity_Int;
double dummy_value = -999;

std::vector<int> nxsec_norm_pointers;
std::vector<std::vector<const double*>> xsec_norm_pointers;

std::vector<int> nxsec_spline_pointers;
std::vector<std::vector<const double*>> xsec_spline_pointers;

std::vector<int> ntotal_weight_pointers;
std::vector<std::vector<const double*>> total_weight_pointers;
std::vector<double> total_w;

std::vector<int> XBin;
std::vector<int> YBin;
std::vector<int> NomXBin;
std::vector<int> NomYBin;

bool *isNC;

// histo pdf bins
Comment on lines +55 to +58
Copy link
Member

Choose a reason for hiding this comment

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

Why is it raw pointer while everyhitng else is vector?

Copy link
Contributor

@luketpickering luketpickering Oct 28, 2024

Choose a reason for hiding this comment

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

Read about std::vector bool, it doesn't behave like other std::vector objects, or just try printing the address of the first three entries of auto myvecbool = std::vector bool {true,true,false};

Copy link
Member

Choose a reason for hiding this comment

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

I feel dumb for not knowing this...

In this case there should be a destructor?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably there should be a default constructor:

FarDetectorCoreInfo() : isNC{nullptr} {}
~FarDetectorCoreInfo(){delete [] isNC;}

std::vector<double> rw_lower_xbinedge; // lower to check if Eb has moved the erec bin
std::vector<double> rw_lower_lower_xbinedge; // lower to check if Eb has moved the erec bin
std::vector<double> rw_upper_xbinedge; // upper to check if Eb has moved the erec bin
std::vector<double> rw_upper_upper_xbinedge; // upper to check if Eb has moved the erec bin

std::vector<double*> mode;

std::vector<const M3::float_t*> osc_w_pointer;
std::vector<double> xsec_w;
};
Loading