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

Add capability for user-defined parameter sets, with an example (take 2) #210

Merged
merged 19 commits into from
Mar 11, 2024

Conversation

glwagner
Copy link
Member

@glwagner glwagner commented Mar 5, 2024

This PR re-opens #172.

This PR tweaks the internal API for defining parameter sets to support user-defined parameters. This is important, because it will allow users to invoke functions in Thermodynamics.jl without requiring the full suite of parameters in ThermodynamicsParameters. For example, if one is only interested in computing density as a function of temperature, pressure, and (total) specific humidity, then they only require the universal gas constant and the molar masses of air and water.

Next, this PR adds an example which illustrates such a parameter set, called ConstitutiveParameters. It culminates in this plot:

image

which uses sea level data derived from the JRA55 reanalysis (subsampled to make the dataset smaller), showing the primary dependence of density on temperature, secondary dependence on pressure, and teritiary dependence on specific humidity for the typical conditions of the Earth's atmosphere at sea level.

Currently, there are no examples, so this PR introduces the first one. I'd be happy to turn the example into something that appears in the documentation via Literate (the example is very close to that form already).

@glwagner glwagner added the Launch Buildkite Add label to launch Buildkite label Mar 5, 2024
Copy link

codecov bot commented Mar 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.93%. Comparing base (46f51af) to head (808b6b6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #210   +/-   ##
=======================================
  Coverage   90.93%   90.93%           
=======================================
  Files          10       10           
  Lines        1180     1180           
=======================================
  Hits         1073     1073           
  Misses        107      107           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glwagner
Copy link
Member Author

glwagner commented Mar 5, 2024

@navidcy @charleskawczynski @simone-silvestri this passes now.

colorrange = Trange,
colormap = Tmap,
alpha = 0.1,
)
Copy link
Member Author

Choose a reason for hiding this comment

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

@charleskawczynski can you please advise on how to make this more readable. It's better if all the arguments appear on one line.

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to enclose it within

#! format: off

#! format: on

Or just a #! format: off at the beginning of the .jl file to make the formatter ignore it altogether?

https://domluna.github.io/JuliaFormatter.jl/dev/#Turn-off/on-formatting

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok that's helpful. But why does the formatter want to do this in the first place?

Copy link
Member

Choose a reason for hiding this comment

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

From my point of view, the formatter is mostly a glorified line-breaker. It's great 95% of the time, but every now and then it's nice to use custom formatting.

I don't think there's an optimal solution for this sort of thing. Maybe something that uses ChatGPT could be a few percent better.

Copy link
Member Author

Choose a reason for hiding this comment

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

But when does it try to break a line?

Copy link
Member

Choose a reason for hiding this comment

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

@charleskawczynski
Copy link
Member

charleskawczynski commented Mar 6, 2024

Can we please rebase and squash this PR? I like having a tidy and linear history.

@glwagner
Copy link
Member Author

glwagner commented Mar 6, 2024

Why?

@charleskawczynski
Copy link
Member

Why?

It's good practice. I can do it. Just let me know when you're ready to merge, @glwagner.

@glwagner
Copy link
Member Author

glwagner commented Mar 8, 2024

I disagree that it's good practice for us. The PR is ready.

src/relations.jl Outdated Show resolved Hide resolved
@navidcy
Copy link
Contributor

navidcy commented Mar 11, 2024

I added the examples in the Docs via Literate.
Feel free to revert if you don't think this is a good idea.

@charleskawczynski charleskawczynski added this pull request to the merge queue Mar 11, 2024
Merged via the queue into main with commit 47b101a Mar 11, 2024
20 checks passed
@glwagner glwagner deleted the glw/density-example branch March 11, 2024 16:16
glwagner added a commit that referenced this pull request Mar 11, 2024
github-merge-queue bot pushed a commit that referenced this pull request Mar 11, 2024
Bump patch version to capture #210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Launch Buildkite Add label to launch Buildkite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants