trixi-framework / trixi-framework/TrixiParticles.jl

Use mandatory keyword arguments instead of positional arguments?

Open
#278 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

discussion
Dominant language
Julia
Stars
83
Forks
24
Avg merge
8d 18h
Merged PRs (30d)
3

Description

In most example files, we have

viscosity = ArtificialViscosityMonaghan(0.02, 0.0)

If you're not familiar with the API, you'd have to check the Julia help to see that the first positional argument is alpha and the second is beta.
In many places, we do this instead:

alpha = 0.02
viscosity = ArtificialViscosityMonaghan(alpha, 0.0)

This makes it clear that the first positional argument is alpha, and it allows us to overwrite this with trixi_include. However, doing this everywhere will create quite a bit of extra code.

We could also use mandatory keyword arguments, so that the line above becomes

viscosity = ArtificialViscosityMonaghan(alpha=0.02, beta=0.0)

Here, we can natively use trixi_include without further modification.

The Trixi.jl API is slowly moving away from positional arguments for exactly this reason. But they often allow both options:
https://github.com/trixi-framework/Trixi.jl/blob/276dc3ca22a4c72b81aa6659e57ff1382dd1ae0c/src/solvers/dgsem/dgsem.jl#L54-L69

Do we want to

  1. keep things like they are?
  2. have keyword arguments instead of positional arguments?
  3. allow both?

I don't like option 3, as it makes the API more complicating by adding more options to do the same thing.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the example files using ArtificialViscosityMonaghan and compare their positional argument usage with Trixi.jl's src/solvers/dgsem/dgsem.jl lines 54-69. Check how trixi_include handles alpha and beta overrides, then establish the preferred constructor style and make the affected examples consistent with that decision.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.