Skip to content

Add clone method #70

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.

Sign up for GitHub

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

Merged
chengcli merged 2 commits into from
Feb 2, 2026
Merged

Add clone method #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3d9ab8
wip
chengcli Jan 27, 2026
3e91bd6
wip
chengcli Jan 28, 2026
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
3 changes: 3 additions & 0 deletions src/kinetics/arrhenius.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ struct ArrheniusOptionsImpl {
virtual std::string name() const { return "arrhenius"; }
virtual ~ArrheniusOptionsImpl() = default;

std::shared_ptr<ArrheniusOptionsImpl> clone() const {
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The clone method should be virtual to support polymorphic cloning. Since ArrheniusOptionsImpl is a polymorphic base class (it has virtual methods like name() and a virtual destructor) and has derived classes like CoagulationOptionsImpl, calling clone() through a base class pointer will cause object slicing if clone() is not virtual.

Suggested change
std::shared_ptr<ArrheniusOptionsImpl> clone() const {
virtual std::shared_ptr<ArrheniusOptionsImpl> clone() const {

Copilot uses AI. Check for mistakes.
return std::make_shared<ArrheniusOptionsImpl>(*this);
}
void report(std::ostream& os) const {
os << "* reactions = " << fmt::format("{}", reactions()) << "\n"
<< "* Tref = " << Tref() << " K\n"
Expand Down
3 changes: 3 additions & 0 deletions src/kinetics/coagulation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ struct CoagulationOptionsImpl final : public ArrheniusOptionsImpl {
CoagulationOptionsImpl(const ArrheniusOptionsImpl& arrhenius)
: ArrheniusOptionsImpl(arrhenius) {}

std::shared_ptr<CoagulationOptionsImpl> clone() const {
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The clone method should override the virtual clone method from the base class and be marked with 'override'. This ensures proper polymorphic behavior and makes the inheritance relationship explicit. Without virtual clone in the base class and override here, cloning through a base class pointer will cause object slicing.

Suggested change
std::shared_ptr<CoagulationOptionsImpl> clone() const {
std::shared_ptr<CoagulationOptionsImpl> clone() const override {

Copilot uses AI. Check for mistakes.
return std::make_shared<CoagulationOptionsImpl>(*this);
}
void report(std::ostream& os) const { ArrheniusOptionsImpl::report(os); }
};
using CoagulationOptions = std::shared_ptr<CoagulationOptionsImpl>;
Expand Down
3 changes: 3 additions & 0 deletions src/kinetics/evaporation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ struct EvaporationOptionsImpl final : public NucleationOptionsImpl {
EvaporationOptionsImpl(const NucleationOptionsImpl& nucleation)
: NucleationOptionsImpl(nucleation) {}

std::shared_ptr<EvaporationOptionsImpl> clone() const {
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The clone method should override the virtual clone method from the base class and be marked with 'override'. This ensures proper polymorphic behavior and makes the inheritance relationship explicit. Without virtual clone in the base class and override here, cloning through a base class pointer will cause object slicing.

Suggested change
std::shared_ptr<EvaporationOptionsImpl> clone() const {
std::shared_ptr<NucleationOptionsImpl> clone() const override {

Copilot uses AI. Check for mistakes.
return std::make_shared<EvaporationOptionsImpl>(*this);
}
void report(std::ostream& os) const {
NucleationOptionsImpl::report(os);
os << "* Tref = " << Tref() << " K\n"
Expand Down
7 changes: 7 additions & 0 deletions src/kinetics/kinetics.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ struct KineticsOptionsImpl final : public SpeciesThermoImpl {
static std::shared_ptr<KineticsOptionsImpl> from_yaml(
YAML::Node const& config, bool verbose = false);

std::shared_ptr<KineticsOptionsImpl> clone() const {
auto op = std::make_shared<KineticsOptionsImpl>(*this);
if (arrhenius()) op->arrhenius() = arrhenius()->clone();
if (coagulation()) op->coagulation() = coagulation()->clone();
if (evaporation()) op->evaporation() = evaporation()->clone();
return op;
}
Comment on lines +36 to +42
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The new clone method lacks test coverage. Given that this is a critical functionality that involves deep copying of nested objects (arrhenius, coagulation, evaporation) and the codebase has comprehensive test coverage for other features, tests should be added to verify that cloning works correctly, especially when these fields contain derived class instances.

Copilot uses AI. Check for mistakes.
void report(std::ostream& os) const {
os << "-- kinetics options --\n";
os << "* Tref = " << Tref() << " K\n"
Expand Down
4 changes: 4 additions & 0 deletions src/kinetics/rate_constant.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ struct RateConstantOptionsImpl {
static std::shared_ptr<RateConstantOptionsImpl> create() {
return std::make_shared<RateConstantOptionsImpl>();
}
std::shared_ptr<RateConstantOptionsImpl> clone() const {
return std::make_shared<RateConstantOptionsImpl>(*this);
}

ADD_ARG(std::vector<std::string>, types) = {};
ADD_ARG(std::string, reaction_file) = "";
};
Expand Down
3 changes: 3 additions & 0 deletions src/thermo/nucleation.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ struct NucleationOptionsImpl {
virtual std::string name() const { return "nucleation"; }
virtual ~NucleationOptionsImpl() = default;

std::shared_ptr<NucleationOptionsImpl> clone() const {
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The clone method should be virtual to support polymorphic cloning. Since NucleationOptionsImpl is a polymorphic base class (it has virtual methods like name() and a virtual destructor) and has derived classes like EvaporationOptionsImpl, calling clone() through a base class pointer will cause object slicing if clone() is not virtual. This means that if a NucleationOptions pointer actually points to an EvaporationOptionsImpl object, calling clone() will only copy the NucleationOptionsImpl portion, losing all EvaporationOptionsImpl-specific data.

Suggested change
std::shared_ptr<NucleationOptionsImpl> clone() const {
virtual std::shared_ptr<NucleationOptionsImpl> clone() const {

Copilot uses AI. Check for mistakes.
return std::make_shared<NucleationOptionsImpl>(*this);
}
void report(std::ostream& os) const {
os << "* reactions = " << fmt::format("{}", reactions()) << "\n"
<< "* minT = " << fmt::format("{}", minT()) << " K\n"
Expand Down
7 changes: 7 additions & 0 deletions src/thermo/thermo.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ struct ThermoOptionsImpl final : public SpeciesThermoImpl {
static std::shared_ptr<ThermoOptionsImpl> from_yaml(YAML::Node const& config,
bool verbose = false);

std::shared_ptr<ThermoOptionsImpl> clone() const {
auto op = std::make_shared<ThermoOptionsImpl>(*this);
if (nucleation() != nullptr) {
op->nucleation() = nucleation()->clone();
}
return op;
}
Comment on lines +58 to +64
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The new clone method lacks test coverage. Given that this is a critical functionality that involves deep copying of nested objects and the codebase has comprehensive test coverage for other features (as seen in the tests directory), tests should be added to verify that cloning works correctly, especially when nucleation contains derived class instances like EvaporationOptionsImpl.

Copilot uses AI. Check for mistakes.
void report(std::ostream& os) const {
os << "-- thermodynamics options --\n";
os << "* Tref = " << Tref() << " K\n"
Expand Down
Loading