Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warning on timesteps written #697

Open
larsbuntemeyer opened this issue Mar 7, 2023 · 6 comments
Open

warning on timesteps written #697

larsbuntemeyer opened this issue Mar 7, 2023 · 6 comments

Comments

@larsbuntemeyer
Copy link

I think this is just a minor issue, but i get a misleading warning when i write several timesteps with cmor.write. For example, using the python5 example, i get the warning:

!!!!!!!!!!!!!!!!!!!!!!!!!
!
! Warning: while closing variable 0 (htovgyre, table Omon)
! we noticed you wrote 0 time steps for the variable,
! but its time axis 0 (time) has 2 time steps
!
!!!!!!!!!!!!!!!!!!!!!!!!!

However, it seems that everything is written correctly and i figured out that i don't get the warning when i pass the number of timesteps explictily, e.g.

cmor.write(cmorVar, data, ntimes_passed=time.size)

However, the documentation states that if omitted, the number will be assumed to be the size of the time dimension of the data (if there is a time dimension). So, this warning is a little misleading because ntimes_passed=time.size should be the default. Just wanted to document it here since it took me some time to figure it out.

@durack1
Copy link
Contributor

durack1 commented Apr 7, 2024

@mauzey1 this seems like a trivial update, I'll label this against the 3.9.0 milestone as it would be useful to cleanup if there is a sensible solution

@taylor13 ping

@taylor13
Copy link
Collaborator

taylor13 commented May 6, 2024

Not sure how trivial, but should be corrected, I think. Could be a problem between python and C???

@mauzey1
Copy link
Collaborator

mauzey1 commented May 13, 2024

I think this issue might be originating from the following section of the function cmor_write_var_to_file. This is the section that writes the time and time bound values from the axis if ntimes_passed is set to zero. Is it assuming that it is writing data for all of the timesteps in the time axis? If the bounds value of the time axis are not null, then it will set the start of the time values to zero otherwise it will be set to the ntimes_written value of the variable. What is the reasoning for this behavior? Shouldn't the start be set to zero for this case whether or not the bounds are not null?

cmor/Src/cmor_variables.c

Lines 3162 to 3248 in 7714266

} else {
/* -------------------------------------------------------------------- */
/* ok we did not pass time values therefore it means they were */
/* defined via the axis */
/* -------------------------------------------------------------------- */
ierr = -1;
/* -------------------------------------------------------------------- */
/* look for time dimension */
/* -------------------------------------------------------------------- */
for (i = 0; i < avar->ndims; i++) {
if (cmor_axes[avar->axes_ids[0]].axis == 'T') {
ierr = i;
break;
}
}
if (ierr != -1) {
if (cmor_axes[avar->axes_ids[ierr]].values == NULL) {
snprintf(msg, CMOR_MAX_STRING,
"variable '%s' (table: %s) you are passing %i "
"times but no values and you did not define "
"them via cmor_axis", avar->id,
cmor_tables[avar->ref_table_id].szTable_id,
ntimes_passed);
cmor_handle_error(msg, CMOR_CRITICAL);
}
avar->first_bound = 1.e20;
avar->last_bound = 1.e20;
if (cmor_axes[avar->axes_ids[ierr]].bounds != NULL) {
/* -------------------------------------------------------------------- */
/* ok at that stage the recentering must already be done so we */
/* just need to write the bounds */
/* -------------------------------------------------------------------- */
counts2[0] = counts[0];
counts2[1] = 2;
starts[0] = 0;
starts[1] = 0;
ierr = nc_put_vara_double(ncid, avar->time_bnds_nc_id, starts,
counts2,
&cmor_axes[avar->
axes_ids[0]].bounds[starts
[0] *
2]);
if (ierr != NC_NOERR) {
snprintf(msg, CMOR_MAX_STRING,
"NCError (%i: %s) writing time bounds values for "
"variable '%s' (table: %s)",
ierr, nc_strerror(ierr), avar->id,
cmor_tables[avar->ref_table_id].szTable_id);
cmor_handle_error(msg, CMOR_CRITICAL);
}
avar->first_bound = cmor_axes[avar->axes_ids[0]].bounds[0];
avar->last_bound = cmor_axes[avar->axes_ids[0]].bounds[counts[0]
* 2 - 1];
}
ierr = nc_put_vara_double(ncid, avar->time_nc_id, starts, counts,
&cmor_axes[avar->
axes_ids[0]].values[starts
[0]]);
if (ierr != NC_NOERR) {
snprintf(msg, CMOR_MAX_STRING,
"NCError (%i: %s) writing time values for "
"variable '%s' (table: %s)",
ierr, nc_strerror(ierr), avar->id,
cmor_tables[avar->ref_table_id].szTable_id);
cmor_handle_error(msg, CMOR_CRITICAL);
}
/* -------------------------------------------------------------------- */
/* ok now we need to store first and last stuff */
/* -------------------------------------------------------------------- */
avar->first_time = cmor_axes[avar->axes_ids[0]].values[0];
avar->last_time = cmor_axes[avar->axes_ids[0]].values[starts[0]
+ counts[0] -
1];
}
}

Near the end of the function outside of the conditional statement for whether ntimes_passed is zero, the ntimes_passed value is added to the variable's ntimes_written value. Should the length of the time axis be added to the ntimes_written value, or should the ntimes_written value be set to the length of the time axis?

avar->ntimes_written += ntimes_passed;

@taylor13
Copy link
Collaborator

Thanks, Chris, for this nice analysis. I'll try to take a look tomorrow at the. coding and hope to understand how we had hoped to handle this situation. If you don't hear back in a couple of days, could you please ping me? thanks.
Karl

@taylor13
Copy link
Collaborator

taylor13 commented May 15, 2024

I really don't know C well enough to be sure, but I think ntimes_written should actually keep track of how many times have been written to the file. In the case that times_passed is missing, then it should be set within the function (and not remain at a default value of 0. I would simply insert after line 3248 something like
ntimes_written = counts[0]
(if counts[0] is the number of time written in this call to the function).

By the way a basic question: If an optional integer argument is missing in a call to a function, does it's value get set to 0?

I'll also try to answer one of your other questions:

** Is it assuming that it is writing data for all of the timesteps in the time axis?

I don't think so ... I think it can write some of the time-steps and then add more timesteps on a later call to the function.

** If the bounds value of the time axis are not null, then it will set the start of the time values to zero otherwise it will be set to the ntimes_written value of the variable. What is the reasoning for this behavior? Shouldn't the start be set to zero for this case whether or not the bounds are not null?

I don't know.

Let me know if you think my suggestion above is consistent with your reasoning.

@mauzey1
Copy link
Collaborator

mauzey1 commented May 16, 2024

@taylor13

By the way a basic question: If an optional integer argument is missing in a call to a function, does it's value get set to 0?

For the C function of cmor_write, you cannot omit the ntimes_passed parameter from the function call. You must set it to zero if you are using the time dimension size for the number of times passed.

For Fortran and Python, omitting this parameter will set it to a default of zero. The length of the time_vals parameter will be used if it is passed without the ntimes_passed parameter.

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

No branches or pull requests

4 participants