Skip to content

Initial XFAIMS modifications and addition of CH3I model - #85

Draft
JanosJiri wants to merge 9 commits into
mainfrom
xfaims
Draft

Initial XFAIMS modifications and addition of CH3I model#85
JanosJiri wants to merge 9 commits into
mainfrom
xfaims

Conversation

@JanosJiri

Copy link
Copy Markdown

I plan to slowly add some basic modifications of XFAIMS and also a simple CH3I model, which allows me to play with it.

@JanosJiri
JanosJiri marked this pull request as draft July 22, 2026 16:56
@JanosJiri
JanosJiri marked this pull request as ready for review July 23, 2026 09:01
@JanosJiri
JanosJiri marked this pull request as draft July 23, 2026 09:14
@JanosJiri

Copy link
Copy Markdown
Author

The tests started failing due to compilation error with make -j. When I compile with make -j1 or just make, everything compiles fine and all tests pass. Even when I do make -j and then make test, the compilation finishes and the tests pass. I don't get any meaningful error so I don't have an idea what is going on. The compilation error also always appears when compiling a different file. Do you have any idea why this is happening @danielhollas ?

@danielhollas

Copy link
Copy Markdown
Member

Hmm, weird, I'll have a look later today.

@danielhollas

Copy link
Copy Markdown
Member

This is the actual error

make[1]: *** No rule to make target modules/xfaimsmodule.mod', needed by openfms.o'. Stop.

You need to add the new module in the module list in src/Makefile

@JanosJiri

Copy link
Copy Markdown
Author

add the new module in the module list in src/Makefile

Cool, thanks!

@codecov-commenter

codecov-commenter commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.38710% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.68%. Comparing base (9f990f2) to head (7ab1f92).

Files with missing lines Patch % Lines
src/modules/XFAIMSModule.f90 4.54% 41 Missing and 1 partial ⚠️
src/modules/ToyModelModule.f90 0.00% 21 Missing ⚠️
src/read_namelist.f90 57.77% 18 Missing and 1 partial ⚠️
src/modules/SpawnModule.f90 54.54% 10 Missing ⚠️
src/modules/ElecStrucModule.f90 0.00% 9 Missing ⚠️
src/modules/OverlapModule.f90 0.00% 4 Missing ⚠️
src/modules/PropagationModule.f90 0.00% 4 Missing ⚠️
src/openfms.F90 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
- Coverage   65.49%   64.68%   -0.81%     
==========================================
  Files          42       43       +1     
  Lines        7511     7617     +106     
  Branches      979      998      +19     
==========================================
+ Hits         4919     4927       +8     
- Misses       2198     2293      +95     
- Partials      394      397       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

A simple two-state CH3I model potential implemented for XFAIMS
calculations. Improvements will probably come later. At the same time,
new XFAIMS module was added, currently containing only one subroutine
which prints all the parameters when XFAIMS is launched.
A new global variable which controls if XFAIMS is active or not was
added. The activation is done in the main dynamics loop in openfms.F90,
because all the other functions and modules, such as FMS_SetTimeStep or
SpawnModule, are called often multiple times in each step. The reduction
of the time step was also messing with the activation and there were two
different condition which were determining XFAIMS activity and time
step, although they were not fully compatible. Activating XFAIMS in the
main look makes everything consistent now. The only disadvantage is that
we can only activate it during the main TimeStep. If the activation time
is not a multiple of TimeStep and the current time step was reduced, we
do not active exactly at the desired time. There is a warning at the
moment. Nevertheless, changing the time step of shifting the whole field
and the activation time would solve everything, just the user needs to
take a minute to do so.
@danielhollas

Copy link
Copy Markdown
Member

All green! 🎉

Btw it would be awesome if as part of this work you could move the xfaims related parameters into the new module and group then into a derived type, similarly to what yorick did with Spawning parameters in #66

This commit tries to unify the printing of the parameters in the input
in a more human readable way such that the user can see them
immediately.
The start of the propagation is not very well/immediatelly visible in
the code.
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.

3 participants