-
Notifications
You must be signed in to change notification settings - Fork 14
Fix material interface, RK3, rename #140
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
Conversation
…ayers (soft landing)
…_material_interface
… in 3D. Added VoidEtching python example. Removed redundant prepareLS(); call in computeRates during advection. Updated RK3 substepping.
…l scheme. The scheme uses a constant velocity during the entire time step, while the gradient is averaged according to SSP-RK3.
…ace. Renamed "timeIntegrationScheme" to "temporalScheme" for consistency with "spatialScheme" naming.
| } | ||
|
|
||
| /// Set which time integration scheme should be used. | ||
| void setTemporalScheme(TemporalSchemeEnum scheme) { temporalScheme = scheme; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The temporal scheme can be set here, but is never accessed. I would either remove this setter or implement some logic to call the corresponding method, since this could be a bit misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I missed this, as I tried different implementations before settling on this one. However, your comment sent me back in my thought process and I have refactored Advect once again to use TemporalSchemeEnum and consolidate time integration into the single class without the need for inherited classes for each additional scheme.
The new commit refactors the Advect class to handle multiple time integration schemes internally, replacing the need for derived classes like AdvectForwardEuler and AdvectRungeKutta3. I have now:
- Actually used
TemporalSchemeEnumwith support for:- Forward Euler
- Runge-Kutta 2nd Order (TVD)
- Runge-Kutta 3rd Order (TVD)
- Moved time integration logic into a new internal helper struct
lsInternal::AdvectTimeIntegrationinlsAdvectTimeIntegration.hpp. - Updated
Advectto dispatchadvect()calls based on the selectedtemporalScheme. - Moved shared utilities (e.g.,
combineLevelSets) into the baseAdvectclass. - Removed
lsAdvectForwardEuler.hppandlsAdvectRungeKutta3.hppas they are now redundant. - Updated Python bindings.
- Updated C++ tests and C++/Python example (AirGapDeposition, TimeIntegrationComparison) to use the new unified API.
include/viennals/lsAdvect.hpp
Outdated
| } else if (spatialScheme == SpatialSchemeEnum::WENO_5TH_ORDER) { | ||
| // WENO5 requires a stencil radius of 3 (template parameter 3) | ||
| lsInternal::WENO5<T, D, 3>::prepareLS(levelSets.back()); | ||
| } else if (spatialScheme == SpatialSchemeEnum::WENO_5TH_ORDER) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WENO5 case is doubled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Fixed in new commit
This pull request includes: