Skip to content

Wave stress quick interp#578

Merged
einola merged 23 commits intodevelopfrom
wave-stress-quick-interp
Jan 5, 2022
Merged

Wave stress quick interp#578
einola merged 23 commits intodevelopfrom
wave-stress-quick-interp

Conversation

@tdcwilliams
Copy link
Copy Markdown
Contributor

Speed up wave coupling (variable exchange down from 20% of total time to 10%)

  • use InterpolationType::FromMeshToMeshQuick for wave_cpl_nodes dataset
  • use InterpolationType::ConservativeRemapping for wave_cpl_elements dataset
  • needed bug fix relating to regridding from 05ddd77
    Also did some cleaning in datasets.cpp and removed unused use_timer option from checkReload[Main]Datasets

Closes #570, PR #577 should be done first

Copy link
Copy Markdown
Member

@einola einola left a comment

Choose a reason for hiding this comment

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

Two comments and one strange commit where a multiplication is replaced with a division. Please double-check that last one.

Comment thread model/externaldata.cpp Outdated
int const cpl_time_last = (cpl_time/cpl_dt)*cpl_dt;
M_dataset->itime_range[0] = cpl_time_last;
M_dataset->itime_range[1] = cpl_time_last + cpl_dt;
//TODO ftime_range not used?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
//TODO ftime_range not used?

ftime_range is used quite a lot: in ExternalData::check_and_reload, ExternalData::value_type, and ExternalData::loadDataset.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

doesn't seem to be used by coupling code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will add a clarification to comment and remove the barrier call

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, yes. When the dataset is the coupling dataset we use itime_range, because the for the coupling code we need integers - it's all in integer time steps. But when the dataset is any other dataset we use ftime_range, because the code for reading in datasets uses floats - it's all in days, if I recall.

What we should do is to use integers for everything - time steps for the coupler and seconds for the datasets - but I think we'll leave that for now!

Comment thread model/externaldata.cpp Outdated
Comment thread model/finiteelement.cpp Outdated
if ( ( it->varID > 0 ) && ( it->cpl_id > 0 ) ) // Skip non-outputing variables
int ierror = OASIS3::put_2d(it->cpl_id, pcpt*time_step, &it->data_grid[0], M_cpl_out.M_ncols, M_cpl_out.M_nrows);
}
M_comm.barrier();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

probably not

tdcwilliams and others added 2 commits December 3, 2021 13:28
fix definition of `ftime_range` (doesn't seem to be used in coupling code though)

Co-authored-by: Einar Örn Ólason <einar.olason@nersc.no>
Comment thread model/finiteelement.cpp Outdated
Comment thread model/externaldata.cpp Outdated
@tdcwilliams
Copy link
Copy Markdown
Contributor Author

hi @einola, your comments addressed and develop merged into this branch - hopefully OK now

Copy link
Copy Markdown
Member

@einola einola left a comment

Choose a reason for hiding this comment

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

Almost there ...

Comment thread model/externaldata.cpp
M_dataset->interpolated = false;
M_dataset->itime_range[0] = cpl_time;
M_dataset->itime_range[1] = cpl_time + cpl_dt;
int const cpl_time_last = (cpl_time/cpl_dt)*cpl_dt;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm sorry, I don't understand why you do this. Both cpl_time and cpl_dt are already integers and cpl_time should always be an integer multiple of cpl_dt. Which means cpl_time_last == cpl_time. Right?

Can you change it back?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that was the main bug - it didn't work when we didn't couple every time step - if a regrid happened in between coupling time steps then it messed up OASIS

@einola einola merged commit 3423baf into develop Jan 5, 2022
@einola einola deleted the wave-stress-quick-interp branch January 30, 2025 05:40
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