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

Fix include ordering for LinearRegression #3541

Merged
merged 3 commits into from Oct 14, 2023

Conversation

rcurtin
Copy link
Member

@rcurtin rcurtin commented Oct 5, 2023

In #3540 @kiner-shah pointed out that it is possible to create a circular include loop! I was surprised by this, but the reason is that the RegressionDistribution class (in src/mlpack/core/dists/) depends on the LinearRegression class (in src/mlpack/methods/). This is the only situation where something in core/ includes something in methods/, and as a result, if you just happen to only include linear_regression.hpp, you get a problem.

Anyway, one solution is to have linear_regression.hpp only include mlpack/prereqs.hpp (which does not include the dists/), and then include mlpack/core.hpp after LinearRegression is defined.

After this PR, the following slightly more complex example compiles:

include <mlpack/methods/linear_regression.hpp>

using namespace mlpack;

int main()
{
  LinearRegression r;
}

@rcurtin rcurtin merged commit bba07a3 into mlpack:master Oct 14, 2023
19 checks passed
@rcurtin rcurtin deleted the fix-lr-includes branch October 14, 2023 19:25
@rcurtin rcurtin mentioned this pull request Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants