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

Include group information in sampling_info for native sampling #1454

Merged
merged 9 commits into from
Jan 22, 2025

Conversation

rthedin
Copy link
Contributor

@rthedin rthedin commented Jan 21, 2025

Summary

Adds additional information for native sampling, written to sampling_info. And switches the file to a yaml format.

Pull request type

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Checklist

The following is included:

  • new unit-test(s)
  • new regression test(s)
  • documentation for new capability

No unit-test or regression tests performed. I couldn't find any specific manual entry regarding this. Maybe we should create one?

Additional background

Now the sampling_info file will look like this

time 100.3
ngroups 2
group_index 0
  name HighT1_inflow0deg
  sampling_type PlaneSampler
group_index 1
  name HighT2_inflow0deg
  sampling_type PlaneSampler

The reader has been updated accordingly, and outputs the info as follows:

{'time': 100.3,
 'ngroups': 2,
 'HighT1_inflow0deg': {'group_index': '0', 'sampling_type': 'PlaneSampler'},
 'HighT2_inflow0deg': {'group_index': '1', 'sampling_type': 'PlaneSampler'}}

This format works well for me, but happy to hear any comments and feedback regarding the format.

@marchdf marchdf self-requested a review January 22, 2025 16:18
@marchdf
Copy link
Contributor

marchdf commented Jan 22, 2025

I made some modifications so the file is now a yaml file. So it will look like this:

time: 0
groups:
 - group_index: 0
   name: line1
   sampling_type: LineSampler

I think this is better future proofing.

@marchdf marchdf enabled auto-merge (squash) January 22, 2025 16:51
@marchdf marchdf disabled auto-merge January 22, 2025 17:00
@marchdf marchdf marked this pull request as draft January 22, 2025 17:00
@marchdf marchdf marked this pull request as ready for review January 22, 2025 18:24
@marchdf marchdf enabled auto-merge (squash) January 22, 2025 18:24
@marchdf marchdf disabled auto-merge January 22, 2025 18:26
@rthedin
Copy link
Contributor Author

rthedin commented Jan 22, 2025

Just to have it documented here, the name of fields have changed since Marc's last comment above. The yaml file looks like this

time: 0
samplers:
 - index: 0
   label: plane1
   type: PlaneSampler
 - index: 1
   label: plane2
   type: PlaneSampler

@marchdf marchdf merged commit cfd3553 into Exawind:main Jan 22, 2025
15 checks passed
@rthedin rthedin deleted the f/samplingfile branch January 22, 2025 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants