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

Earlier signal creation #782

Merged
merged 4 commits into from Mar 6, 2016
Merged

Conversation

tjhei
Copy link
Member

@tjhei tjhei commented Mar 4, 2016

This patch moves the creation of the signals object closer to the top of the simulator and call the connector functions as soon as possible (instead of at the end of the initialization). In the future, this will allow us to fire new signals during introspection construction for example. Right now this doesn't do anything exciting.

  • connect signals earlier in the program
  • catch when you try to register after connector functions have already been called
  • update tests

@tjhei
Copy link
Member Author

tjhei commented Mar 4, 2016

this is extracted from #772

void call_connector_functions (aspect::SimulatorSignals<2> &signals);
void call_connector_functions (aspect::SimulatorSignals<3> &signals);
template <int dim>
void call_connector_functions (aspect::SimulatorSignals<dim> &signals_);
Copy link
Member

Choose a reason for hiding this comment

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

Is this underscore necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't. The name is silent anyway.

@gassmoeller
Copy link
Member

I like it. Except for the one naming issue this seems good to go. I would suggest you wait over the weekend in case @bangerth has comments, but I would say feel free to merge afterwards. Not many controversial things anyway.

@@ -72,6 +56,8 @@ namespace aspect
const unsigned int max_part_per_cell,
const unsigned int weight)
{
World<dim>::connect_to_signals(this->get_signals());
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to qualify the class name here

@bangerth
Copy link
Contributor

bangerth commented Mar 5, 2016

Please address the minor issues and merge.

tjhei added a commit that referenced this pull request Mar 6, 2016
@tjhei tjhei merged commit d280a57 into geodynamics:master Mar 6, 2016
@tjhei tjhei deleted the earlier_signal_creation branch March 6, 2016 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants