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.

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

Jump to bottom

Draft PR: Remove solver_.parameters_ #668

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Draft PR: Remove solver_.parameters_ #668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions include/micm/solver/solver.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ namespace micm
return solver_.Solve(time_step, state);
}

// Overloaded Solve function to change parameters
SolverResult Solve(double time_step, StatePolicy& state, RosenbrockSolverParameters params)
{
solver_.parameters_.h_start_ = params.h_start_;
return solver_.Solve(time_step, state);
}
Comment on lines +67 to +71
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SolverResult Solve(double time_step, StatePolicy& state, RosenbrockSolverParameters params)
{
solver_.parameters_.h_start_ = params.h_start_;
return solver_.Solve(time_step, state);
}
SolverResult Solve(double time_step, StatePolicy& state, RosenbrockSolverParameters params)
{
solver_.parameters_ = params;
return solver_.Solve(time_step, state);
}

We actually need all of the paramters to be updated. That was the impetus for moving the tolerances off of the params. Also, this needs to work for any set of parameters, not just rosenbrock


/// @brief Returns the number of grid cells
/// @return
std::size_t GetNumberOfGridCells() const
Expand Down
4 changes: 2 additions & 2 deletions test/unit/cuda/solver/test_cuda_rosenbrock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,12 +284,12 @@ TEST(RosenbrockSolver, SingularSystemZeroInBottomRightOfU)
// so H needs to be 1 / ( (-k1 - k2) * gamma)
// since H is positive we need -k1 -k2 to be positive, hence the smaller, negative value for k1
double H = 1 / ((-k1 - k2) * params.gamma_[0]);
vector_solver.solver_.parameters_.h_start_ = H;
params.h_start_ = H;

vector_solver.CalculateRateConstants(vector_state);
vector_state.SyncInputsToDevice();

auto vector_result = vector_solver.Solve(2 * H, vector_state);
auto vector_result = vector_solver.Solve(2 * H, vector_state, params);
vector_state.SyncOutputsToHost();
EXPECT_NE(vector_result.stats_.singular_, 0);
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit/jit/solver/test_jit_rosenbrock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ TEST(JitRosenbrockSolver, SingularSystemZeroInBottomRightOfU)
// so H needs to be 1 / ( (-k1 - k2) * gamma)
// since H is positive we need -k1 -k2 to be positive, hence the smaller, negative value for k1
double H = 1 / ((-k1 - k2) * params.gamma_[0]);
vector_solver.solver_.parameters_.h_start_ = H;
params.h_start_ = H;

vector_solver.CalculateRateConstants(vector_state);

auto vector_result = vector_solver.Solve(2 * H, vector_state);
auto vector_result = vector_solver.Solve(2 * H, vector_state, params);
EXPECT_NE(vector_result.stats_.singular_, 0);
}

Expand Down
11 changes: 5 additions & 6 deletions test/unit/solver/test_rosenbrock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ void testNormalizedErrorDiff(SolverBuilderPolicy builder, std::size_t number_of_
{
builder = getSolver(builder);
auto solver = builder.SetNumberOfGridCells(number_of_grid_cells).Build();
std::vector<double> atol = solver.solver_.parameters_.absolute_tolerance_;
std::vector<double> atol = solver.solver_.parameters_.absolute_tolerance_;
double rtol = solver.solver_.parameters_.relative_tolerance_;

auto state = solver.GetState();
Expand Down Expand Up @@ -172,18 +172,17 @@ TEST(RosenbrockSolver, SingularSystemZeroInBottomRightOfU)
// alpha is 1 / (H * gamma), where H is the time step and gamma is the gamma value from
// the rosenbrock paramters
// so H needs to be 1 / ( (-k1 - k2) * gamma)
// since H is positive we need -k1 -k2 to be positive, hence the smaller, negative value for k1
// since H is positive we need -k1 -k2 to be positive, hence the smaller, negative value for k1
double H = 1 / ((-k1 - k2) * params.gamma_[0]);
standard_solver.solver_.parameters_.h_start_ = H;
vector_solver.solver_.parameters_.h_start_ = H;
params.h_start_ = H;

standard_solver.CalculateRateConstants(standard_state);
vector_solver.CalculateRateConstants(vector_state);

auto standard_result = standard_solver.Solve(2 * H, standard_state);
auto standard_result = standard_solver.Solve(2 * H, standard_state, params);
EXPECT_NE(standard_result.stats_.singular_, 0);

auto vector_result = vector_solver.Solve(2 * H, vector_state);
auto vector_result = vector_solver.Solve(2 * H, vector_state, params);
EXPECT_NE(vector_result.stats_.singular_, 0);
}

Expand Down
Loading