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

Remove deprecated Laplacian API #1834

Merged
merged 6 commits into from
Nov 15, 2019
Merged

Remove deprecated Laplacian API #1834

merged 6 commits into from
Nov 15, 2019

Conversation

ZedThree
Copy link
Member

@ZedThree ZedThree commented Nov 6, 2019

No description provided.

@ZedThree ZedThree added the breaking change Introduces a change that is not backward compatible with the previous major version label Nov 6, 2019
@ZedThree ZedThree added this to the BOUT-5.0 milestone Nov 6, 2019

low_mem = (*options)["low_mem"]
.doc("If true, reduce the amount of memory used")
.withDefault(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like low_mem is only actually implemented now by LaplacePDD. If we remove that (#1828) we can remove the option too.

}
int flags = (*options)["flags"]
.doc("Flags to control inner and outer boundaries.")
.withDefault(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with removing this option in v5, but it might be a good idea to put a deprecation warning on use of the laplace:flags option in v4.3.1/v4.4. I can't think of anything better than printing a message to output_warn here, which isn't very noticable, but better than nothing...

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea! I think there's a few options we'd like to deprecate like this too, although none spring to mind right now.

OPTION(options, inner_boundary_flags, 0);
OPTION(options, outer_boundary_flags, 0);
}
global_flags = (*options)["global_flags"].doc("Default flags").withDefault(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

"Default flags" seems a bit vague, although I'm struggling to come up with something more descriptive - maybe something like "Flags to set Laplace solver options (apart from boundary conditions)"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Very true. I just took the docstring from the header.

@ZedThree ZedThree merged commit 2e377e4 into next Nov 15, 2019
@ZedThree ZedThree deleted the remove-invert-laplace branch November 15, 2019 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Introduces a change that is not backward compatible with the previous major version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants