Skip to content

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

Merged
yantosca merged 4 commits intodevfrom
bugfix/yin-placement-in-update-rconst
Dec 20, 2024
Merged

Add code from #INLINE F90_RCONST_USE at the top of the UPDATE_RCONST subroutine#120
yantosca merged 4 commits intodevfrom
bugfix/yin-placement-in-update-rconst

Conversation

@yantosca
Copy link
Copy Markdown
Contributor

@yantosca yantosca commented Dec 19, 2024 •
edited
Loading

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:

  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

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

@yantosca
Now inline "INLINED RCONST" at the top of routine "UPDATE_RCONST"
2a6b3fa 
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]>
@yantosca 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 yantosca added this to the 3.2.0 milestone Dec 19, 2024
@yantosca yantosca requested a review from jimmielin December 19, 2024 21:54
@yantosca yantosca self-assigned this Dec 19, 2024
@yantosca yantosca requested a review from RolfSander December 19, 2024 21:54
@yantosca 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
Closed
@yantosca
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
Copy link
Copy Markdown
Contributor

RolfSander commented Dec 20, 2024

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
RolfSander requested changes Dec 20, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@RolfSander RolfSander left a comment

Choose a reason for hiding this comment

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

Maybe create F90_RCONST_USE?

@yantosca Octobox
Copy link
Copy Markdown
Contributor Author

yantosca commented Dec 20, 2024 •
edited
Loading

@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!

@yantosca
Added new #INLINE F90_RCONST_USE option for F90 USE statements
bd4745f 
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 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
Copy link
Copy Markdown
Contributor Author

yantosca commented Dec 20, 2024 •
edited
Loading

@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.

@yantosca yantosca requested a review from RolfSander December 20, 2024 17:08
RolfSander
RolfSander approved these changes Dec 20, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@RolfSander RolfSander left a comment

Choose a reason for hiding this comment

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

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

  1. It would be good to mention the new inline type also in
    docs/source/using_kpp/04_input_for_kpp.rst.

  2. 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?

  3. 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
Copy link
Copy Markdown
Contributor Author

yantosca commented Dec 20, 2024

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

@yantosca
Add minor fixes to PR #120 suggested by @RolfSander
2f97825 
.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
Copy link
Copy Markdown
Contributor Author

yantosca commented Dec 20, 2024

Minor fixes based on your suggestions added in commit 2f97825

@yantosca yantosca merged commit 64e2744 into dev Dec 20, 2024
@yantosca yantosca deleted the bugfix/yin-placement-in-update-rconst branch December 20, 2024 20:32
@yantosca yantosca mentioned this pull request Jan 27, 2025
Closed
@yantosca yantosca mentioned this pull request Feb 28, 2025
Merged
@yantosca yantosca mentioned this pull request Oct 27, 2025
Open
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RolfSander RolfSander RolfSander approved these changes

@jimmielin jimmielin Awaiting requested review from jimmielin

Assignees

@yantosca yantosca

Labels

bugfix Fixes a bug or a technical issue code / structural Related to structural source code updates target-languages Related to the language options for KPP-generated code

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

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

2 participants

@yantosca @RolfSander