Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine #120

yantosca · 2024-12-19T21:54:40Z

This is the companion PR to #119. In that issue we described how the UPDATE_RCONST routine was generated such that a USE statement in INLINED RCONST:

SUBROUTINE Update_RCONST ( YIN )

! YIN - Optional input concentrations of variable species
  REAL(kind=dp), OPTIONAL :: YIN(NVAR)

! Y - Concentrations of species (local)
  REAL(kind=dp) :: Y(NSPEC)

! Ensure local Y array is filled with variable and constant concentrations
  Y(1:NSPEC) = C(1:NSPEC)

! Update local Y array if variable concentrations are provided
  if (present(YIN)) Y(1:NVAR) = YIN(1:NVAR)


! Begin INLINED RCONST


  ! Inline an include file containing rate law definitions, which
  ! will be inserted directly into subroutine Update_Rconst().
  ! This is necessary as a workaround for KPP not being able to
  ! include very large files ( > 200000 chars) directly.
  !  -- Bob Yantosca (11 Jun 2021)
  USE fullchem_RateLawFuncs

! End INLINED RCONST

could cause a compiler error due to the USE statement not being first in the subroutine.

This PR has edited the GenerateUpdateRconst routine in src/gen.c to do the following:

Manually write the SUBROUTINE UPDATE_RCONST ( YIN ) line
Inline code from the #INLINE F90_RCONST_USE section
Declare YIN optional argument and Y local variable
Copy values of YIN to Y (if necessary)
Inline code from the #INLINE F90_RCONST section
Manually write the END SUBROUTINE UPDATE_RCONST line

This will only affect the F90 output, which now looks like this:

SUBROUTINE Update_RCONST ( YIN )

! Begin INLINED RCONST - F90 USE STATEMENTS


  ! Inline an include file containing rate law definitions, which
  ! will be inserted directly into subroutine Update_Rconst().
  ! This is necessary as a workaround for KPP not being able to
  ! include very large files ( > 200000 chars) directly.
  !  -- Bob Yantosca (11 Jun 2021)
  USE fullchem_RateLawFuncs

! End INLINED RCONST - F90 USE STATEMENTS

! YIN - Optional input concentrations of variable species
  REAL(kind=dp), OPTIONAL :: YIN(NVAR)

! Y - Concentrations of species (local)
  REAL(kind=dp) :: Y(NSPEC)

! Ensure local Y array is filled with variable and constant concentrations
  Y(1:NSPEC) = C(1:NSPEC)

! Update local Y array if variable concentrations are provided
  if (present(YIN)) Y(1:NVAR) = YIN(1:NVAR)




! Begin INLINED RCONST


! End INLINED RCONST

  RCONST(1) = ...

which now compiles properly:

KPP has successfully created the model "gckpp".

gfortran -cpp -O  -c gckpp_Precision.F90
gfortran -cpp -O  -c gckpp_Parameters.F90
gfortran -cpp -O  -c gckpp_Global.F90
gfortran -cpp -O  -c gckpp_JacobianSP.F90
gfortran -cpp -O  -c gckpp_Jacobian.F90
gfortran -cpp -O  -c gckpp_Function.F90
gfortran -cpp -O  -c rateLawUtilFuncs.F90
gfortran -cpp -O  -c fullchem_RateLawFuncs.F90
gfortran -cpp -O  -c gckpp_Rates.F90
gfortran -cpp -O  -c gckpp_Monitor.F90
gfortran -cpp -O  -c gckpp_Util.F90
gfortran -cpp -O  -c gckpp_LinearAlgebra.F90
gckpp_LinearAlgebra.F90:3455:46:

 3455 |       IF (incX .EQ. incY) IF (incX-1) 5,20,60
      |                                              1
Warning: Fortran 2018 deleted feature: Arithmetic IF statement at (1)
make: Circular gckpp_Model.o <- gckpp_Model.o dependency dropped.
gfortran -cpp -O  -c gckpp_Initialize.F90
make: Circular gckpp_Integrator.o <- gckpp_Model.o dependency dropped.
gfortran -cpp -O  -c gckpp_Integrator.F90
gfortran -cpp -O  -c gckpp_Model.F90
gfortran -cpp -O  -c kpp_standalone_init.F90
gfortran -cpp -O  -c kpp_standalone.F90
gfortran -cpp -O  kpp_standalone.F90 gckpp_Integrator.o kpp_standalone_init.o gckpp_Precision.o gckpp_Parameters.o gckpp_Global.o      gckpp_JacobianSP.o    gckpp_Jacobian.o gckpp_Function.o    gckpp_Rates.o   gckpp_Util.o   gckpp_Monitor.o fullchem_RateLawFuncs.o rateLawUtilFuncs.o gckpp_LinearAlgebra.o    gckpp_Model.o gckpp_Initialize.o    -o kpp_standalone.exe

Closes #119

src/gen.c - Modified routine GenerateUpdateRconst so that the INLINED RCONST contents will be placed immedately after the start of the UPDATE_RCONST subroutine, and to place F90 variable declarations etc. immediately after that. This will prevent compile-time errors if a USE statement is included via "INLINED RCONST", as USE statements must come before variable declarationas and other statements. - Updated comments CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <[email protected]>

src/gen.c - Removed leftover comment about "int UPDATE_RCONST;" variable - Fixed typo Signed-off-by: Bob Yantosca <[email protected]>

RolfSander · 2024-12-20T11:22:56Z

We have this problem in the dev branch since SUBROUTINE Update_RCONST
has a parameter. Unfortunately, I don't think that the solution covers
all cases. In our docs, we present two possibilities for using INLINE F90_RCONST:

#INLINE F90_RCONST
  USE MyRateFunctionModule
#ENDINLINE

and

#INLINE F90_RCONST
  k_DMS_OH = 1.E-9*EXP(5820./temp)*C(ind_O2)/ &
    (1.E30+5.*EXP(6280./temp)*C(ind_O2))
#ENDINLINE

The solution in this PR works fine in the first case when the
F90_RCONST block contains only USE statements. However, after
applying this PR, the code fails when variables are defined directly, as
in the second case.

A possible solution could be to create a separate inlined type
called F90_RCONST_USE where the user can place USE statements.

RolfSander

Maybe create F90_RCONST_USE?

yantosca · 2024-12-20T14:17:37Z

@RolfSander: Yes, I thought of that after I went home after making the PR. I'll try to add a separate inline for USE statements. Thanks for the feedback!

src/gdata.h - Declare F90_RCONST_USE as a variable of type "enum inl_code" src/scanner.c - Added F90_RCONST_USE to InlineKeys[] src/gen.c - In routine GenerateUpdateRconst we now proceed in this order: 1. Manually write the "SUBROUTINE UPDATE_RCONST ( YIN )" line 2. Inline code from the "#INLINE F90_RCONST_USE" section 3. Declare YIN optional argument and Y local variable 4. Copy values of YIN to Y (if necessary) 5. Inline code from the "#INLINE F90_RCONST" section 6. Manually write the "END SUBROUTINE UPDATE_RCONST" line .gitignore - Now ignore executable files in MCM example folders CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <[email protected]>

yantosca · 2024-12-20T17:04:53Z

@RolfSander: I modified the code in bd4745f so that we place #INLINE F90_RCONST_USE at the top of the subroutine while leaving the #INLINE F90_RCONST untouched. I've also updated the initial comments above to reflect this.

RolfSander

Thanks, @yantosca for coding this so quickly! The code looks good, I
just have a few minor comments:

It would be good to mention the new inline type also in
docs/source/using_kpp/04_input_for_kpp.rst.
You've added /examples/mcm*/*.exe to .gitignore. I wonder if
there are any executable *.exe files at all that we want in our
repo. If not, maybe we can exclude all *.exe files in .gitignore?
Currently, the following comment is inserted into the KPP-generated
files:

Begin INLINED RCONST - F90 USE STATEMENTS

Maybe it would be better to rename this to:

Begin inlined code from F90_RCONST_USE

Using the term "F90_RCONST_USE" verbatim here would facilitate
finding the inlined code in the KPP-generated files. Similarly, I
think it would be good if the name appears verbatim also in the
comments created for the other F90_* inline types.

If you agree but don't have the time to implement it now, let me know!

yantosca · 2024-12-20T20:09:59Z

Thanks @RolfSander. I will make those edits before I merge the branch. Easy enough!

.gitignore - Ignore all *.exe files src/gen.c - Updated commments where inlined code is added to be more descriptive (referencing F90_RCONST_USE and F90_RCONST) docs/source/using_kpp/04_input_for_kpp.rst - Added documentation for F90_RCONST_USE CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <[email protected]>

yantosca · 2024-12-20T20:31:00Z

Minor fixes based on your suggestions added in commit 2f97825

yantosca added code / structural Related to structural source code updates target-languages Related to the language options for KPP-generated code bugfix Fixes a bug or a technical issue labels Dec 19, 2024

yantosca added this to the 3.2.0 milestone Dec 19, 2024

yantosca requested a review from jimmielin December 19, 2024 21:54

yantosca self-assigned this Dec 19, 2024

yantosca requested a review from RolfSander December 19, 2024 21:54

yantosca linked an issue Dec 19, 2024 that may be closed by this pull request

[BUG/ISSUE] Wrong placement of YIN argument in subroutine Update_Rconst #119

Open

Updated comments in src/gen.c

0da6983

src/gen.c - Removed leftover comment about "int UPDATE_RCONST;" variable - Fixed typo Signed-off-by: Bob Yantosca <[email protected]>

RolfSander requested changes Dec 20, 2024

View reviewed changes

yantosca changed the title ~~Now place "INLINED RCONST" content at the top of routine "UPDATE_RCONST"~~ Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine Dec 20, 2024

yantosca requested a review from RolfSander December 20, 2024 17:08

RolfSander approved these changes Dec 20, 2024

View reviewed changes

yantosca merged commit 64e2744 into dev Dec 20, 2024
1 check passed

yantosca deleted the bugfix/yin-placement-in-update-rconst branch December 20, 2024 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine #120

Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine #120

yantosca commented Dec 19, 2024 •

edited

Loading

RolfSander commented Dec 20, 2024

RolfSander left a comment

yantosca commented Dec 20, 2024 •

edited

Loading

yantosca commented Dec 20, 2024 •

edited

Loading

RolfSander left a comment

yantosca commented Dec 20, 2024

yantosca commented Dec 20, 2024

Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine #120

Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine #120

Conversation

yantosca commented Dec 19, 2024 • edited Loading

RolfSander commented Dec 20, 2024

RolfSander left a comment

Choose a reason for hiding this comment

yantosca commented Dec 20, 2024 • edited Loading

yantosca commented Dec 20, 2024 • edited Loading

RolfSander left a comment

Choose a reason for hiding this comment

yantosca commented Dec 20, 2024

yantosca commented Dec 20, 2024

yantosca commented Dec 19, 2024 •

edited

Loading

yantosca commented Dec 20, 2024 •

edited

Loading

yantosca commented Dec 20, 2024 •

edited

Loading