-
Notifications
You must be signed in to change notification settings - Fork 3
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
Vector-ordered Rosenbrock Solver #172
Changes from all commits
4ac0fab
e27634b
d114885
6d2bc7d
bf57538
090b620
062c748
4546e4a
fa82486
54ec3e7
b362330
30bfafc
4c10839
b484cfe
2d50377
f4f6526
b8481f6
5a67815
983a510
997038b
8b08448
80a0f66
23889c3
276bc8f
2812b21
09bf9b8
680a2cb
61c87df
63bf319
15ae988
e53faf2
1f548de
830b426
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
FROM fedora:35 | ||
|
||
ARG BUILD_TYPE=release | ||
ARG BUILD_TYPE=Release | ||
|
||
RUN dnf -y update \ | ||
&& dnf -y install \ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ module musica_micm | |
implicit none | ||
|
||
public :: micm_t, solver_stats_t, get_micm_version | ||
public :: Rosenbrock, RosenbrockStandardOrder | ||
private | ||
|
||
!> Wrapper for c solver stats | ||
|
@@ -25,13 +26,22 @@ module musica_micm | |
real(c_double) :: final_time_ = 0._c_double | ||
end type solver_stats_t_c | ||
|
||
! We could use Fortran 2023 enum type feature if Fortran 2023 is supported | ||
! https://fortran-lang.discourse.group/t/enumerator-type-in-bind-c-derived-type-best-practice/5947/2 | ||
enum, bind(c) | ||
enumerator :: Rosenbrock = 1 | ||
enumerator :: RosenbrockStandardOrder = 2 | ||
end enum | ||
Comment on lines
+29
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As long as our CI tests pass then I'm fine with this. The only potential issue maybe is the NAG compiler and if it supports this and if we need to compile with NAG ever. But I like this as it is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Jesse mentioned that it's probably not yet to use Fortran 2023 features but I'll leave the comment so we can revisit later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this too |
||
|
||
interface | ||
function create_micm_c(config_path, error) bind(C, name="CreateMicm") | ||
function create_micm_c(config_path, solver_type, num_grid_cells, error) bind(C, name="CreateMicm") | ||
use musica_util, only: error_t_c | ||
import c_ptr, c_int, c_char | ||
character(kind=c_char), intent(in) :: config_path(*) | ||
type(error_t_c), intent(inout) :: error | ||
type(c_ptr) :: create_micm_c | ||
character(kind=c_char), intent(in) :: config_path(*) | ||
integer(kind=c_int), value, intent(in) :: solver_type | ||
integer(kind=c_int), value, intent(in) :: num_grid_cells | ||
type(error_t_c), intent(inout) :: error | ||
type(c_ptr) :: create_micm_c | ||
end function create_micm_c | ||
|
||
subroutine delete_micm_c(micm, error) bind(C, name="DeleteMicm") | ||
|
@@ -182,10 +192,12 @@ function get_micm_version() result(value) | |
value = string_t(string_c) | ||
end function get_micm_version | ||
|
||
function constructor(config_path, error) result( this ) | ||
function constructor(config_path, solver_type, num_grid_cells, error) result( this ) | ||
use musica_util, only: error_t_c, error_t, copy_mappings | ||
type(micm_t), pointer :: this | ||
character(len=*), intent(in) :: config_path | ||
integer(c_int), intent(in) :: solver_type | ||
integer(c_int), intent(in) :: num_grid_cells | ||
type(error_t), intent(inout) :: error | ||
character(len=1, kind=c_char) :: c_config_path(len_trim(config_path)+1) | ||
integer :: n, i | ||
|
@@ -201,7 +213,7 @@ function constructor(config_path, error) result( this ) | |
end do | ||
c_config_path(n+1) = c_null_char | ||
|
||
this%ptr = create_micm_c(c_config_path, error_c) | ||
this%ptr = create_micm_c(c_config_path, solver_type, num_grid_cells, error_c) | ||
error = error_t(error_c) | ||
if (.not. error%is_success()) then | ||
deallocate(this) | ||
|
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.
Hi @mattldawson and @K20shores, what do you think about this change? Matt and I briefly discussed about this. The issue is that
OpenMP::OpenMP_Fortran
is not found whentest_micm_c_api
is built. I removed it because this line belongs tofunction(create_standard_test_cxx)
and I assumed that it shouldn't needOpenMp_Fortran
for c++ tests.Matt mentioned we could specify fortran as one of the languages in the top-level
CMakeLists.txt
and updatefind_package
incmake/dependencies.cmake
I think that makes sense although I think it is useful to separate the fortran stuff from
MUSICA-C
as it is since we eventually pursue fortran-free. I'd like to hear what you all think.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.
I would like to keep fortran separated, but with the inclusion of tuvx into the musica-c library that line is blurred. Because of that I think we should go with Matt's suggestion. The other option is to check if TUVX is enabled before we check for OpenMP. If it is, enable the fortran language so that the fortran part of OpenMP is found, if that's possible
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.
I created a separate issue to tackle this.