Apply the modifications of https://github.com/libprima/prima/pull/135 to Aeq, beq, Aineq, and bineq?

[zaikunzhang]

Hi @nbelakovski ,

I have gone through #135 and made some revisions to the algorithm_c.f90 files:

9880e58

It is mainly to

• add deallocation of allocatable variables (without this, Valgrind would complain about memory leak)
• divide "Read the inputs and convert them to the Fortran side types" into two groups: complementary arguments and optional arguments
• add comments about unallocated variables, pointing to related sections in the Fortran standard
• ordered the local variables

Do they seem OK to you?

In addition, I have got a question: Should we apply the modifications of PR 135 to Aeq, beq, Aineq, and bineq? See

prima/c/lincoa_c.f90

Lines 92 to 95 in 439019f

	Aineq_loc = real(transpose(Aineq), kind(Aineq_loc))
	bineq_loc = real(bineq, kind(bineq_loc))
	Aeq_loc = real(transpose(Aeq), kind(Aeq_loc))
	beq_loc = real(beq, kind(beq_loc))

prima/c/cobyla_c.f90

Lines 100 to 103 in 439019f

	Aineq_loc = real(transpose(Aineq), kind(Aineq_loc))
	bineq_loc = real(bineq, kind(bineq_loc))
	Aeq_loc = real(transpose(Aeq), kind(Aeq_loc))
	beq_loc = real(beq, kind(beq_loc))

[zaikunzhang]

BTW, does the following initialization set all integers in prima_options_t to 0 and all pointers to NULL? Is this guaranteed? Thanks.

    memset(options, 0, sizeof(prima_options_t));

[nbelakovski]

Do they seem OK to you?

A couple comments:

1. You have a comment that reads See Sec. 9.7.1.3 and 15.5.2.13 of J3/24-007 (Fortran 2023 Interpretation Document). I think maybe we should reference the Fortran 2008 standard since the goal of the project is to be compatible with 2008, right? I just wouldn't want anyone to read this comment and walk away thinking that this code must be compiled with the 2023 standard.
2. You have a comment that reads Deallocate variables not needed any more. Indeed, automatic allocation will take place at exit.. Did you mean automatic deallocation? If so, then (unless I misunderstand Fortran) automatic deallocation isn't considered a valid cleanup strategy and shouldn't be relied upon. It's true that if I write a C program like this one:

   int main(int argc, char * argv[]) {
     int *x = malloc(8);
     FILE *fp = open("somefile.txt", 'r');
     return 0;
   }

   then yes, x will be freed before the program exits (or I guess after it exits?) and the file fp will be closed and from a purely practical point of view it doesn't make any difference if I call free and close before return 0;. However as a "responsible person" the "right thing to do" is to call free and close before return 0;. The point I'm making here is that the second sentence in your comment, the one that starts with "Indeed," is unnecessary and can be removed.
3. In lincoa_c.f90 the line that starts ! initialization to null because it implies ... was accidentally deleted. The whole comment is missing entirely in uobyqa_c.f90. One day we should make a unified interface in Fortran so we don't have to write almost the same code 5 times.

In addition, I have got a question: Should we apply the modifications of PR 135 to Aeq, beq, Aineq, and bineq?

I think maybe it's not necessary because we rely on m_eq and m_ineq to set the size of the corresponding matrices/vectors in Fortran, and if they're 0 (which is their default value), then that shouldn't cause any problems right? I haven't actually looked at how Fortran handles 0 length vectors or nx0 matrices, but I haven't seen any segfaults from this code yet so I just assumed it's fine.

BTW, does the following initialization set all integers in prima_options_t to 0 and all pointers to NULL? Is this guaranteed? Thanks.

Yes it sets all integers to 0 and all pointers to NULL and yes this is guaranteed.

[zaikunzhang]

1. You have a comment that reads See Sec. 9.7.1.3 and 15.5.2.13 of J3/24-007 (Fortran 2023 Interpretation Document). I think maybe we should reference the Fortran 2008 standard since the goal of the project is to be compatible with 2008, right? I just wouldn't want anyone to read this comment and walk away thinking that this code must be compiled with the 2023 standard.

PRIMA aims to be compatible with F2008 and above. This means that the code compiles without any warning using any compiler with fstd=xyz (or something equivalent), where xyz >= 2008 (not only xyz = 2008). The code should not use any construct that does not exist in F2008, and should exploit the new features of F2008 whenever appropriate.

When referring to the Fortran standard for some feature, I always use the latest standard (F2023 as of today). If the standards are inconsistent about this feature, then PRIMA should not use it at all (I do not this has ever happened, as Fortran is super conservative about backward compatibility). Since the standards are consistent, referring to the latest one is better.

[zaikunzhang]

2. You have a comment that reads Deallocate variables not needed any more. Indeed, automatic allocation will take place at exit.. Did you mean automatic deallocation? If so, then (unless I misunderstand Fortran) automatic deallocation isn't considered a valid cleanup strategy and shouldn't be relied upon.

See https://fortran-lang.discourse.group/t/best-practice-deallocating-allocatable-arrays-explicitly-vs-implicitly.

This is a feature of modern Fortran that I avoid using. Another related one is automatic (re)allocation. I believe explicit allocation and deallocation are better, exactly due to the explicitness. In addition, explicit deallocation enables us to free the memory as soon as possible, which is beneficial if the memory under discussion is huge.

Thanks.

[zaikunzhang]

In lincoa_c.f90 the line that starts ! initialization to null because it implies ... was accidentally deleted. The whole comment is missing entirely in uobyqa_c.f90.

Fixed: 1bdb6ea

One day we should make a unified interface in Fortran so we don't have to write almost the same code 5 times.

I thought about it before. Currently, I have no plan for this.

[nbelakovski]

2. You have a comment that reads Deallocate variables not needed any more. Indeed, automatic allocation will take place at exit.. Did you mean automatic deallocation? If so, then (unless I misunderstand Fortran) automatic deallocation isn't considered a valid cleanup strategy and shouldn't be relied upon.

See https://fortran-lang.discourse.group/t/best-practice-deallocating-allocatable-arrays-explicitly-vs-implicitly.

This is a feature of modern Fortran that I avoid using. Another related one is automatic (re)allocation. I believe explicit allocation and deallocation are better, exactly due to the explicitness. In addition, explicit deallocation enables us to free the memory as soon as possible, which is beneficial if the memory under discussion is huge.

Thanks.

I believe you misunderstood my comment. I was making two points

1. You say automatic allocation will... and I wondered if this was a typo and perhaps you meant automatic DEallocation will ...?
2. Since both you and I agree that explicit deallocation is better, the comment doesn't make a lot of sense. If you're choosing to explicitly deallocate variables, why would you tell the reader that they will be implicitly deallocated anyway? Perhaps you want to say something like "We prefer explicit deallocation even though automatic deallocation will take place at function exit"?