Skip to content

Parallel netcdf: colstat, virtualmeasurement and warmstart fixes - #302

Open
Xetalim wants to merge 22 commits into
dalesteam:parallel-netcdffrom
Xetalim:parallel-netcdf
Open

Parallel netcdf: colstat, virtualmeasurement and warmstart fixes#302
Xetalim wants to merge 22 commits into
dalesteam:parallel-netcdffrom
Xetalim:parallel-netcdf

Conversation

@Xetalim

@Xetalim Xetalim commented Jun 26, 2026

Copy link
Copy Markdown

To be merged in parallel netcdf. Contains some cleanup for modstat_nc, and (optional) chunking behavior.

Xetalim added 9 commits June 26, 2026 13:28
plus chunking options.. sorry.

I checked modcolstat by taking a column profile at every single point, and comparing the average of all columns with modgenstat. differences were ~epsilon for all variables
if dt_lim is tnext, it  currently works at a coldstart, but at a warmstart, dt_lim would be (t_warmstart + dt_lim) instead of dt_lim.
also make sure dt_lim is correctly set at start for new netcdf interface
@Xetalim
Xetalim marked this pull request as draft June 26, 2026 12:19
@Xetalim

Xetalim commented Jun 26, 2026

Copy link
Copy Markdown
Author

To be continued after the weekend...

@Xetalim
Xetalim marked this pull request as ready for review July 1, 2026 09:28
@Xetalim

Xetalim commented Jul 1, 2026

Copy link
Copy Markdown
Author

I've fixed the SP compilation, but I don't really like the solution. If any of you have a better idea of how to do this, let me know!

@Xetalim

Xetalim commented Jul 7, 2026

Copy link
Copy Markdown
Author

Last commit should have fixed all SP issues, so should be ready for review :)

Comment thread src/modcrosssection.f90 Outdated
crossplane(2:100) = -999
crossortho(1) = 2
crossortho(2:100) = -999
crossheight(1)=2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why remove spaces???

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point, I had originally removed this initialization towards cross*(1)=2, but that broke the default behaviour, so I brought it back (wrongly). Fixed now!

Comment thread src/modlsmdata.f90
! Monin-obukhov / surface layer:
real, allocatable :: obuk(:,:), ustar(:,:), ra(:,:)
real(field_r), allocatable :: obuk(:,:)
real, allocatable :: ustar(:,:), ra(:,:)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can't these be field_r as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, but we might want to separate that out in a different PR? Cleaning up some of the extra field_r stuff

Comment thread src/modstat_nc_files.f90 Outdated
!> Loop over the files and write those that are due.
subroutine write_output_files()

use modtimer, only: timer_tic, timer_toc

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please put this at the top of the module with the other use statements :-)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I moved them :)

Comment thread src/modstat_nc_files.f90 Outdated

call timer_tic("write_output_files")
do ifile = 1, nfiles
call timer_tic("write_output_files_file_"//trim(file_list(ifile)%file%filename))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ahhh, timers per file. I like it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I like them as well but I unfortunately had to remove them, because when you use crosssections, it might be that 1 core has more timers than another, which leads to a hang in the timer writes at the end :/

Comment thread src/modsurface.f90 Outdated
if(zeta <= 0) then
x = (1. - 16. * zeta) ** (0.25)
psim = 3.14159265 / 2. - 2. * atan(x) + log( (1.+x) ** 2. * (1. + x ** 2.) / 8.)
x = (1._field_r - 16._field_r * zeta) ** (0.25_field_r)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Integers don't have to be casted to field_r explicitly, the compiler will take care of this. It's not wrong, but not doint it can keep lines like these a bit shorter and more readable

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I changed them back to normal integers, seems like a good idea

Comment thread src/modtimestat.f90
u0av_patch = patchsum_1level_field_r(u0(2:i1,2:j1,k)) * (xpatches*ypatches/ijtot)
v0av_patch = patchsum_1level_field_r(v0(2:i1,2:j1,k)) * (xpatches*ypatches/ijtot)
w0av_patch = patchsum_1level_field_r(w0(2:i1,2:j1,k)) * (xpatches*ypatches/ijtot)
u0av_patch = patchsum_1level(u0(2:i1,2:j1,k)) * (xpatches*ypatches/ijtot)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The lhetero stuff is already gone in dev, so this might lead to merge conflicts.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think it's better to have the merge conflict later, as it's nice to know that it compiles correctly on this branch, as not doing anything there will lead to SP comp errors

@CasparJungbacker CasparJungbacker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have one general question about the new statistics modules: why do you setup (allocatable) variables for all quantities, instead of using pointers and the get_pointer method from the various NetCDF file type wrappers?

@Xetalim

Xetalim commented Jul 16, 2026

Copy link
Copy Markdown
Author

I moved towards using the pointer manager instead. The only possibly nasty thing is that the scalar value calculations have to get a new pointer each time we move to a different column index.

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.

2 participants