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

Switched MGCycle to sg by default #212

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Switched MGCycle to sg by default #212

merged 1 commit into from
Mar 29, 2022

Conversation

ewu63
Copy link
Collaborator

@ewu63 ewu63 commented Mar 28, 2022

Purpose

Closes #191. I was thinking about actually listing out the possible choices for this option, but a quick glance at the Fortran code shows that this may be a bit complicated to do. Maybe the current description is okay.

Expected time until merged

Should be pretty quick.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

None

Checklist

  • I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@ewu63 ewu63 requested a review from a team as a code owner March 28, 2022 15:31
@ewu63 ewu63 requested review from anilyil and ArshSaja March 28, 2022 15:31
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #212 (d8a76c6) into main (0a9fe1b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #212   +/-   ##
=======================================
  Coverage   43.32%   43.32%           
=======================================
  Files          15       15           
  Lines        3550     3550           
=======================================
  Hits         1538     1538           
  Misses       2012     2012           
Impacted Files Coverage Δ
adflow/pyADflow.py 67.45% <ø> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@marcomangano marcomangano left a comment

Choose a reason for hiding this comment

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

I am fine with having just this change for now, maybe we could look deeper into the options as we deal with #206 ?

@sseraj
Copy link
Collaborator

sseraj commented Mar 28, 2022

We can't put all possible options into a list, but we could be more clear that the input is an integer specifying the number of grid levels and then w or v specifying the type of cycle. We also have the option to manually describe a cycle with a sequence of 0, -1, 1 but I don't know if anyone uses that.

@anilyil anilyil merged commit 1f33c41 into main Mar 29, 2022
@anilyil anilyil deleted the update-MGCycle branch March 29, 2022 00:57
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.

Update default options
4 participants