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

Simplify examples #229

Merged
merged 5 commits into from
Apr 14, 2021
Merged

Simplify examples #229

merged 5 commits into from
Apr 14, 2021

Conversation

navidcy
Copy link
Member

@navidcy navidcy commented Apr 14, 2021

This PR simplifies the examples. Now common functions from FFTW and CUDA packages are re-exported by FourierFlows. Also, FourierFlows namespace is reexported by GeophysicalFlows. Thus, using GeophysicalFlows suffices for running simulations. The users still need to load, e.g.,Plots.jl if they want to plot the output.

(Inspired by @pdebuyl's remark in JOSS paper review.)

Closes #33.

@navidcy navidcy changed the title Ncc/reexport fourierflows Simplify examples Apr 14, 2021
@navidcy navidcy requested a review from glwagner April 14, 2021 05:30
Copy link
Member

@glwagner glwagner left a comment

Choose a reason for hiding this comment

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

Looks good.

Someday we may be able to combine functions like energy defined in different modules. We have to figure out a way to determine which physical problem a FourierFlows.Problem refers to (a simple way to achieve this would be to add type parameters to FourierFlows.Problem that identify the module it comes from...)

@navidcy navidcy merged commit bafe2bc into master Apr 14, 2021
@navidcy
Copy link
Member Author

navidcy commented Apr 16, 2021

I guess if we do something like

abstract type TwoDNavierStokesProblem <: AbstractProblem end

and define the Problem struct in FourierFlows.jl somehow to get the above subtype as a parameter?

Then we can have

energy(prob::TwoDNavierStokesProblem) = ...

@navidcy navidcy deleted the ncc/reexport-fourierflows branch April 26, 2021 02:53
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.

@reexport using FourierFlows in GeophysicalFlows.jl?
2 participants