Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF

What changes does this PR introduce?

We want to be able to add to TAF_COMMON_FLAGS via genmake_local
changes to genmake2 to enable mixed .F and .F90 code for TAF

What is the current behaviour?
(You can also link to an open issue here)
What is the new behaviour
(if this is a feature change)?
Does this PR introduce a breaking change?
(What changes might users need to make in their application due to this PR?)
Other information:
Suggested addition to tag-index
(To avoid unnecessary merge conflicts, please don't update tag-index. One of the admins will do that when merging your pull request.)

Hi Patrick, I always wanted to to be able to use both *.F90 and F. files.
But I tried (on my arm64 MacBook with gfortran) :
 ./testreport -t isomip -adm -devel
...
  N/O   isomip.htd

the taf output in make.tr_log starts with
file ad_input_code.for:
     1 cadj SUBROUTINE active_read_xy FTLNAME = g_active_read_xy
            ^
*ERROR* : = expected
                       ^
*ERROR* : end of line expected
     2 cadj SUBROUTINE active_read_xy ADNAME  = adactive_read_xy
            ^

and I tried
./testreport -t isomip -adm -ncad -devel
...
  N/O   isomip.htd

Here, TAF does not seem to be called at all leading to a missing _adthe_main_loop_ in the link step.
Assuming that this works for you, do you have any idea which parts of the changes may cause this?

With these changes, it seems to work for me also with MacOS. @heimbach do you want me to push this, or would you rather try this first.
modified   tools/genmake2
@@ -1817,7 +1817,9 @@ if test "x${AD_OPTFILE}" != xNONE ; then
 	exit 1
     fi
 fi
-### if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+# this seems to be necessary because we now mix .F and .F90 files
+TAF_EXTRA="${TAF_EXTRA} -fixed"

 #====================================================================
 # Initialize INCLUDEDIRSMK from and optfile INCLUDEDIRS
@@ -3339,10 +3341,8 @@ remove_comments:
 	sed -i.bak -f \$(TOOLSDIR)/remove_comments_sed \$(AD_FILES)
 	@-rm -f \$(AD_FILES:.$FS=.$FS.bak)
 adj_sed:
-	sed -i.bak -f \$(TOOLSDIR)/adjoint_sed \$(AD_FILES:.$FS=_ad.$FS)
-	@-rm -f \$(AD_FILES:.$FS=_ad.$FS.bak)
-	sed -i.bak -f \$(TOOLSDIR)/adjoint_sed \$(AD_F90FILES:.$FS90=_ad.$FS90)
-	@-rm -f \$(AD_F90FILES:.$FS90=_ad.$FS90.bak)
+	sed -i.bak -f \$(TOOLSDIR)/adjoint_sed \$(AD_FILES:.$FS=_ad.$FS) \$(AD_F90FILES:.$FS90=_ad.$FS90)
+	@-rm -f \$(AD_FILES:.$FS=_ad.$FS.bak) \$(AD_F90FILES:.$FS90=_ad.$FS90.bak)
 ftl_sed:
 	sed -i.bak -f \$(TOOLSDIR)/adjoint_sed \$(AD_FILES:.$FS=_tl.$FS)
 	@-rm -f \$(AD_FILES:.$FS=_tl.$FS.bak)
@@ -3416,11 +3416,10 @@ ad_taf_output.$FS: \$(AD_FLOW_FILES) \$(AD_FILES) \$(AD_F90FILES)
 	@\$(MAKE) -f \$(MAKEFILE) \$(F77_PP_SRC_FILES)
 	@\$(MAKE) -f \$(MAKEFILE) \$(F90_PP_SRC_FILES)
 	@\$(MAKE) -f \$(MAKEFILE) \$(FLOWFILES)
-	cat \$(FLOWFILES) > all_flowfiles.$FS
 	@-rm -f \$(AD_FILES:.$FS=_ad.$FS) \$(AD_FILES:.$FS=_ad.o); echo ''
 	@-rm -f \$(AD_F90FILES:.$FS90=_ad.$FS90) \$(AD_F90FILES:.$FS90=_ad.o); echo ''
 	\$(MAKE) -f \$(MAKEFILE) remove_comments
-	\$(TAF) \$(AD_TAF_FLAGS) \$(TAF_EXTRA) all_flowfiles.$FS \$(AD_FILES) \$(AD_F90FILES)
+	\$(TAF) \$(AD_TAF_FLAGS) \$(TAF_EXTRA) \$(FLOWFILES) \$(AD_FILES) \$(AD_F90FILES)
 	\$(MAKE) -f \$(MAKEFILE) adj_sed
 	cat \$(AD_FILES:.$FS=_ad.$FS) > ad_taf_output.$FS

Martin,
Very quick responses between meetings:
I may misunderstand you, but we need to omit the “-fixed” option to be able to differentiate “true” (i.e., free-format) .F90 files. So for true .F90 files, “-fixed” must not be used. I also am not sure why this is needed at all, but probably simply can’t remember the reason.
As far as I can tell, the mixed .f, .f90 case can ONLY work with the “-ncad” option. In this way, TAF will automatically treat .f files as fixed format and .f90 files as free-format files (as it should). Not using “-ncad” would require concatenating .f90 files into a separate ad_input_code.f90 for TAF to treat those files properly.
The changes I made to genmake2 are not sufficient to run the mixed format case. For the test I used (more below), I added a genmake_local file in which I add two more changes:
1/ add TAF option “-e” in adoptfile
2/ add two compiler options in optfile that extend lines to 132 (this one may ultimately not be needed, but worked for TAF-generated code)
I have not tested on MacOS, only linux box. For item 2/ above I only initially added the gfortran compiler option. Will be good to make it generic.
I agree we should develop a verification for this ASAP. What I tested is not yet fully generic but getting there.
Here’s what I tested (on linux box, with gfortran):
https://github.com/christophernhill/gmao_mitgcm_couplng/tree/master/geos_bulkformulae/global_ocean.cs32x15
The code compiled successfully (including TAF) for the following steps (from global_ocean.cs32x15):
cd build_geos_ad
../../../tools/genmake2 -ncad -mods '../geos_blkf ../code_geos_ad'
make depend
make adtaf
make adall
Notice the genmake_local file in build_geos_ad/ to change options in adjoint_default
Also note that some flowdirs/keys  in the geos_bulkf code are still incorrect (haven’t sorted the correct keys yet).
Another important change was that we need to now treat all flowdir files as .f files (since they are in F77 format), i.e., you’ll now see these lines in genmake2
cat $(FLOWFILES) > all_flowfiles.$FS
$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) all_flowfiles.$FS $(AD_FILES) $(AD_F90FILES)
I ran testreport on lab_sea with the modified genmake2 and it worked for me (that was with the default, which I guess does not use -nocad). Quite likely that I missed something though. I’ll be happy to test isomip later and see why it does not work.
Does that help?
p.
From: Martin Losch @.>
Date: Thursday, March 9, 2023 at 6:09 AM
To: MITgcm/MITgcm @.>
Cc: @.*** @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
With these changes, it seems to work for me also with MacOS. @heimbachhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fheimbach&data=05|01||c030b1a3eefc44e52cbd08db20973334|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139605990311324|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=fth%2FCQ9AtTupSf4FcJjJz16QR7rvtU5WyyX0TPr3f8A%3D&reserved=0 do you want me to push this, or would you rather try this first.
modified   tools/genmake2
@@ -1817,7 +1817,9 @@ if test "x${AD_OPTFILE}" != xNONE ; then
exit 1
 fi

fi
-### if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+# this seems to be necessary because we now mix .F and .F90 files
+TAF_EXTRA="${TAF_EXTRA} -fixed"
#====================================================================
Initialize INCLUDEDIRSMK from and optfile INCLUDEDIRS
@@ -3339,10 +3341,8 @@ remove_comments:
sed -i.bak -f $(TOOLSDIR)/remove_comments_sed $(AD_FILES)
@-rm -f $(AD_FILES:.$FS=.$FS.bak)
adj_sed:

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_ad.$FS)

@-rm -f $(AD_FILES:.$FS=_ad.$FS.bak)

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_F90FILES:.$FS90=_ad.$FS90)

@-rm -f $(AD_F90FILES:.$FS90=_ad.$FS90.bak)

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_ad.$FS) $(AD_F90FILES:.$FS90=_ad.$FS90)

@-rm -f $(AD_FILES:.$FS=_ad.$FS.bak) $(AD_F90FILES:.$FS90=_ad.$FS90.bak)

ftl_sed:
sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_tl.$FS)
@-rm -f $(AD_FILES:.$FS=_tl.$FS.bak)
@@ -3416,11 +3416,10 @@ ad_taf_output.$FS: $(AD_FLOW_FILES) $(AD_FILES) $(AD_F90FILES)
@$(MAKE) -f $(MAKEFILE) $(F77_PP_SRC_FILES)
@$(MAKE) -f $(MAKEFILE) $(F90_PP_SRC_FILES)
@$(MAKE) -f $(MAKEFILE) $(FLOWFILES)

cat $(FLOWFILES) > all_flowfiles.$FS
@-rm -f $(AD_FILES:.$FS=_ad.$FS) $(AD_FILES:.$FS=_ad.o); echo ''
@-rm -f $(AD_F90FILES:.$FS90=_ad.$FS90) $(AD_F90FILES:.$FS90=_ad.o); echo ''
$(MAKE) -f $(MAKEFILE) remove_comments

$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) all_flowfiles.$FS $(AD_FILES) $(AD_F90FILES)

$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) $(FLOWFILES) $(AD_FILES) $(AD_F90FILES)
$(MAKE) -f $(MAKEFILE) adj_sed
cat $(AD_FILES:.$FS=_ad.$FS) > ad_taf_output.$FS

—
Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMITgcm%2FMITgcm%2Fpull%2F709%23issuecomment-1461921310&data=05|01||c030b1a3eefc44e52cbd08db20973334|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139605990311324|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=cZ58JKjiJTqzwj7WHUZwWpMSnDrC8BCRHEGk0od3hrk%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABHJNDJFYPFPKOOLVNWPHXDW3HCA7ANCNFSM6AAAAAAVURWB4I&data=05|01||c030b1a3eefc44e52cbd08db20973334|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139605990467570|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=NQ76Geg7gDAYV4tdqOvMnFSNBV9T9giYAvA06Egl7hg%3D&reserved=0.
You are receiving this because you were mentioned.Message ID: @.***>

PS: Two more notes:
Perhaps prudent in the below test case to specify gfortran explicitly, ie
../../../tools/genmake2 -fc gfortran -ncad -mods '../geos_blkf ../code_geos_ad'
(to re-emphasize, the change to FFLAGS and F90FLAGS in genmake_local may not ultimately be needed, can explain later why they were needed here).
The relevant mix of .F and F.90 codes used in the test is in
https://github.com/christophernhill/gmao_mitgcm_couplng/tree/master/geos_bulkformulae/global_ocean.cs32x15/geos_blkf
From: Heimbach, Patrick @.>
Date: Thursday, March 9, 2023 at 8:31 AM
To: MITgcm/MITgcm @.>, MITgcm/MITgcm @.>
Cc: @. @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
Martin,
Very quick responses between meetings:
I may misunderstand you, but we need to omit the “-fixed” option to be able to differentiate “true” (i.e., free-format) .F90 files. So for true .F90 files, “-fixed” must not be used. I also am not sure why this is needed at all, but probably simply can’t remember the reason.
As far as I can tell, the mixed .f, .f90 case can ONLY work with the “-ncad” option. In this way, TAF will automatically treat .f files as fixed format and .f90 files as free-format files (as it should). Not using “-ncad” would require concatenating .f90 files into a separate ad_input_code.f90 for TAF to treat those files properly.
The changes I made to genmake2 are not sufficient to run the mixed format case. For the test I used (more below), I added a genmake_local file in which I add two more changes:
1/ add TAF option “-e” in adoptfile
2/ add two compiler options in optfile that extend lines to 132 (this one may ultimately not be needed, but worked for TAF-generated code)
I have not tested on MacOS, only linux box. For item 2/ above I only initially added the gfortran compiler option. Will be good to make it generic.
I agree we should develop a verification for this ASAP. What I tested is not yet fully generic but getting there.
Here’s what I tested (on linux box, with gfortran):
https://github.com/christophernhill/gmao_mitgcm_couplng/tree/master/geos_bulkformulae/global_ocean.cs32x15https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fchristophernhill%2Fgmao_mitgcm_couplng%2Ftree%2Fmaster%2Fgeos_bulkformulae%2Fglobal_ocean.cs32x15&data=05|01||dffba0ea7e4f4a15af1e08db20aafd8a|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139691012920652|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=HpUG5kTeqP1Ye2L%2FKuem0FMh5hWq2%2FXXQ16X2kNId%2BM%3D&reserved=0
The code compiled successfully (including TAF) for the following steps (from global_ocean.cs32x15):
cd build_geos_ad
../../../tools/genmake2 -ncad -mods '../geos_blkf ../code_geos_ad'
make depend
make adtaf
make adall
Notice the genmake_local file in build_geos_ad/ to change options in adjoint_default
Also note that some flowdirs/keys  in the geos_bulkf code are still incorrect (haven’t sorted the correct keys yet).
Another important change was that we need to now treat all flowdir files as .f files (since they are in F77 format), i.e., you’ll now see these lines in genmake2
cat $(FLOWFILES) > all_flowfiles.$FS
$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) all_flowfiles.$FS $(AD_FILES) $(AD_F90FILES)
I ran testreport on lab_sea with the modified genmake2 and it worked for me (that was with the default, which I guess does not use -nocad). Quite likely that I missed something though. I’ll be happy to test isomip later and see why it does not work.
Does that help?
p.
From: Martin Losch @.>
Date: Thursday, March 9, 2023 at 6:09 AM
To: MITgcm/MITgcm @.>
Cc: @.*** @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
With these changes, it seems to work for me also with MacOS. @heimbachhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fheimbach&data=05|01||dffba0ea7e4f4a15af1e08db20aafd8a|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139691012920652|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=uBr%2BWha1y6aQQ1m5rLYN4vLEaoPJHJUdV37891eAB0M%3D&reserved=0 do you want me to push this, or would you rather try this first.
modified   tools/genmake2
@@ -1817,7 +1817,9 @@ if test "x${AD_OPTFILE}" != xNONE ; then
exit 1
 fi

fi
-### if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+# this seems to be necessary because we now mix .F and .F90 files
+TAF_EXTRA="${TAF_EXTRA} -fixed"
#====================================================================
Initialize INCLUDEDIRSMK from and optfile INCLUDEDIRS
@@ -3339,10 +3341,8 @@ remove_comments:
sed -i.bak -f $(TOOLSDIR)/remove_comments_sed $(AD_FILES)
@-rm -f $(AD_FILES:.$FS=.$FS.bak)
adj_sed:

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_ad.$FS)

@-rm -f $(AD_FILES:.$FS=_ad.$FS.bak)

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_F90FILES:.$FS90=_ad.$FS90)

@-rm -f $(AD_F90FILES:.$FS90=_ad.$FS90.bak)

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_ad.$FS) $(AD_F90FILES:.$FS90=_ad.$FS90)

@-rm -f $(AD_FILES:.$FS=_ad.$FS.bak) $(AD_F90FILES:.$FS90=_ad.$FS90.bak)

ftl_sed:
sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_tl.$FS)
@-rm -f $(AD_FILES:.$FS=_tl.$FS.bak)
@@ -3416,11 +3416,10 @@ ad_taf_output.$FS: $(AD_FLOW_FILES) $(AD_FILES) $(AD_F90FILES)
@$(MAKE) -f $(MAKEFILE) $(F77_PP_SRC_FILES)
@$(MAKE) -f $(MAKEFILE) $(F90_PP_SRC_FILES)
@$(MAKE) -f $(MAKEFILE) $(FLOWFILES)

cat $(FLOWFILES) > all_flowfiles.$FS
@-rm -f $(AD_FILES:.$FS=_ad.$FS) $(AD_FILES:.$FS=_ad.o); echo ''
@-rm -f $(AD_F90FILES:.$FS90=_ad.$FS90) $(AD_F90FILES:.$FS90=_ad.o); echo ''
$(MAKE) -f $(MAKEFILE) remove_comments

$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) all_flowfiles.$FS $(AD_FILES) $(AD_F90FILES)

$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) $(FLOWFILES) $(AD_FILES) $(AD_F90FILES)
$(MAKE) -f $(MAKEFILE) adj_sed
cat $(AD_FILES:.$FS=_ad.$FS) > ad_taf_output.$FS

—
Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMITgcm%2FMITgcm%2Fpull%2F709%23issuecomment-1461921310&data=05|01||dffba0ea7e4f4a15af1e08db20aafd8a|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139691012920652|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=Vm%2BGobsu%2BhATTj4Mp22a%2FwZt8QsaVxeDrQDKPeP1SJQ%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABHJNDJFYPFPKOOLVNWPHXDW3HCA7ANCNFSM6AAAAAAVURWB4I&data=05|01||dffba0ea7e4f4a15af1e08db20aafd8a|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139691012920652|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=uAHyRmTNM3AjyexVkSwjqtDdeFys7CTkAIQpgwPCpO4%3D&reserved=0.
You are receiving this because you were mentioned.Message ID: @.***>

PS2:
@mlosch The lines I added in genmake2

     sed -i.bak -f \$(TOOLSDIR)/adjoint_sed \$(AD_F90FILES:.$FS90=_ad.$FS90)
     @-rm -f \$(AD_F90FILES:.$FS90=_ad.$FS90.bak)

are not needed and will cause adjoint verifications with pure F77 code to fail when using the -ncad flag.
Removing those two lines should fix your isomip AD test with -ncad.
From: Heimbach, Patrick @.>
Date: Thursday, March 9, 2023 at 8:51 AM
To: MITgcm/MITgcm @.>, MITgcm/MITgcm @.>
Cc: @. @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
PS: Two more notes:
Perhaps prudent in the below test case to specify gfortran explicitly, ie
../../../tools/genmake2 -fc gfortran -ncad -mods '../geos_blkf ../code_geos_ad'
(to re-emphasize, the change to FFLAGS and F90FLAGS in genmake_local may not ultimately be needed, can explain later why they were needed here).
The relevant mix of .F and F.90 codes used in the test is in
https://github.com/christophernhill/gmao_mitgcm_couplng/tree/master/geos_bulkformulae/global_ocean.cs32x15/geos_blkfhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fchristophernhill%2Fgmao_mitgcm_couplng%2Ftree%2Fmaster%2Fgeos_bulkformulae%2Fglobal_ocean.cs32x15%2Fgeos_blkf&data=05|01||0e11bf3280904ad202e108db20adcaa8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139703053603716|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=Md4nY5jPpEXLuOnou%2BqeMqc%2BH9wL1DeoL399NWZbhl0%3D&reserved=0
From: Heimbach, Patrick @.>
Date: Thursday, March 9, 2023 at 8:31 AM
To: MITgcm/MITgcm @.>, MITgcm/MITgcm @.>
Cc: @. @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
Martin,
Very quick responses between meetings:
I may misunderstand you, but we need to omit the “-fixed” option to be able to differentiate “true” (i.e., free-format) .F90 files. So for true .F90 files, “-fixed” must not be used. I also am not sure why this is needed at all, but probably simply can’t remember the reason.
As far as I can tell, the mixed .f, .f90 case can ONLY work with the “-ncad” option. In this way, TAF will automatically treat .f files as fixed format and .f90 files as free-format files (as it should). Not using “-ncad” would require concatenating .f90 files into a separate ad_input_code.f90 for TAF to treat those files properly.
The changes I made to genmake2 are not sufficient to run the mixed format case. For the test I used (more below), I added a genmake_local file in which I add two more changes:
1/ add TAF option “-e” in adoptfile
2/ add two compiler options in optfile that extend lines to 132 (this one may ultimately not be needed, but worked for TAF-generated code)
I have not tested on MacOS, only linux box. For item 2/ above I only initially added the gfortran compiler option. Will be good to make it generic.
I agree we should develop a verification for this ASAP. What I tested is not yet fully generic but getting there.
Here’s what I tested (on linux box, with gfortran):
https://github.com/christophernhill/gmao_mitgcm_couplng/tree/master/geos_bulkformulae/global_ocean.cs32x15https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fchristophernhill%2Fgmao_mitgcm_couplng%2Ftree%2Fmaster%2Fgeos_bulkformulae%2Fglobal_ocean.cs32x15&data=05|01||0e11bf3280904ad202e108db20adcaa8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139703053759934|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=kKcR%2BRrOzqnzSw0%2BG37C%2BIbuDbspm0rRnr6fuGQKTDI%3D&reserved=0
The code compiled successfully (including TAF) for the following steps (from global_ocean.cs32x15):
cd build_geos_ad
../../../tools/genmake2 -ncad -mods '../geos_blkf ../code_geos_ad'
make depend
make adtaf
make adall
Notice the genmake_local file in build_geos_ad/ to change options in adjoint_default
Also note that some flowdirs/keys  in the geos_bulkf code are still incorrect (haven’t sorted the correct keys yet).
Another important change was that we need to now treat all flowdir files as .f files (since they are in F77 format), i.e., you’ll now see these lines in genmake2
cat $(FLOWFILES) > all_flowfiles.$FS
$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) all_flowfiles.$FS $(AD_FILES) $(AD_F90FILES)
I ran testreport on lab_sea with the modified genmake2 and it worked for me (that was with the default, which I guess does not use -nocad). Quite likely that I missed something though. I’ll be happy to test isomip later and see why it does not work.
Does that help?
p.
From: Martin Losch @.>
Date: Thursday, March 9, 2023 at 6:09 AM
To: MITgcm/MITgcm @.>
Cc: @.*** @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
With these changes, it seems to work for me also with MacOS. @heimbachhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fheimbach&data=05|01||0e11bf3280904ad202e108db20adcaa8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139703053759934|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=XJrHSC%2BsINVHWOj7%2B6J3K9bkk1wN2ZfIA8IvR6fUFlE%3D&reserved=0 do you want me to push this, or would you rather try this first.
modified   tools/genmake2
@@ -1817,7 +1817,9 @@ if test "x${AD_OPTFILE}" != xNONE ; then
exit 1
 fi

fi
-### if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+if test $CAT_SRC_FOR_TAF = 0 ; then TAF_EXTRA="${TAF_EXTRA} -fixed" ; fi
+# this seems to be necessary because we now mix .F and .F90 files
+TAF_EXTRA="${TAF_EXTRA} -fixed"
#====================================================================
Initialize INCLUDEDIRSMK from and optfile INCLUDEDIRS
@@ -3339,10 +3341,8 @@ remove_comments:
sed -i.bak -f $(TOOLSDIR)/remove_comments_sed $(AD_FILES)
@-rm -f $(AD_FILES:.$FS=.$FS.bak)
adj_sed:

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_ad.$FS)

@-rm -f $(AD_FILES:.$FS=_ad.$FS.bak)

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_F90FILES:.$FS90=_ad.$FS90)

@-rm -f $(AD_F90FILES:.$FS90=_ad.$FS90.bak)

sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_ad.$FS) $(AD_F90FILES:.$FS90=_ad.$FS90)

@-rm -f $(AD_FILES:.$FS=_ad.$FS.bak) $(AD_F90FILES:.$FS90=_ad.$FS90.bak)

ftl_sed:
sed -i.bak -f $(TOOLSDIR)/adjoint_sed $(AD_FILES:.$FS=_tl.$FS)
@-rm -f $(AD_FILES:.$FS=_tl.$FS.bak)
@@ -3416,11 +3416,10 @@ ad_taf_output.$FS: $(AD_FLOW_FILES) $(AD_FILES) $(AD_F90FILES)
@$(MAKE) -f $(MAKEFILE) $(F77_PP_SRC_FILES)
@$(MAKE) -f $(MAKEFILE) $(F90_PP_SRC_FILES)
@$(MAKE) -f $(MAKEFILE) $(FLOWFILES)

cat $(FLOWFILES) > all_flowfiles.$FS
@-rm -f $(AD_FILES:.$FS=_ad.$FS) $(AD_FILES:.$FS=_ad.o); echo ''
@-rm -f $(AD_F90FILES:.$FS90=_ad.$FS90) $(AD_F90FILES:.$FS90=_ad.o); echo ''
$(MAKE) -f $(MAKEFILE) remove_comments

$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) all_flowfiles.$FS $(AD_FILES) $(AD_F90FILES)

$(TAF) $(AD_TAF_FLAGS) $(TAF_EXTRA) $(FLOWFILES) $(AD_FILES) $(AD_F90FILES)
$(MAKE) -f $(MAKEFILE) adj_sed
cat $(AD_FILES:.$FS=_ad.$FS) > ad_taf_output.$FS

—
Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMITgcm%2FMITgcm%2Fpull%2F709%23issuecomment-1461921310&data=05|01||0e11bf3280904ad202e108db20adcaa8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139703053759934|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=o%2Ffw8E4astI3ZOUJpNMZiLybdPfH4qpScyww%2FPAulGo%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABHJNDJFYPFPKOOLVNWPHXDW3HCA7ANCNFSM6AAAAAAVURWB4I&data=05|01||0e11bf3280904ad202e108db20adcaa8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139703053759934|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=sS4kv2MgOqi%2BbklKcBboKbfoL%2B1oC7uZCJLdN0wbq7g%3D&reserved=0.
You are receiving this because you were mentioned.Message ID: @.***>

@heimbach I took the liberty to edit your posts (stripped the original messages that the emails replied to) to make it easier to follow this thread.
Thanks for the explanations, I now understand more of the changes. Is still work in progress, or is this already ready for review? If it is work in progress, we should add the corresponding label or make it a draft PR and I will hold off any further amateur attempts to make this work for me (it seems that make and Makefiles are my personal enemy).
Otherwise, I am not sure what to do here. The first thing I normally do is check if this PR breaks anything. I just used isomip, because it is usually fast to process (for taf), but the problem is the same for all other experiments. Also with lab_sea, the target adj_sed fails. When I fix this (see above just for adj_sed: move \$(AD_F90FILES:.$FS90=_ad.$FS90) to the same line as $(AD_FILES:.$FS=_ad.$FS)), the lab_sea experiment compiles on a linux box (but not on my Mac, because the target adtaf is not made. I need to remove all_flowfiles.$FS to make it work there, and I am not sure why the -fixed flag is necessary, either. Let's leave the Mac issue for now), but it stops with this error (on linux):
>  MDS_READ_FIELD: filename: ./ctrl_variables/adxx_atemp.0000000000 , ./ctrl_variables/adxx_atemp.0000000000.001.001.data
>  MDS_READ_FIELD: Files DO not exist
>  additional DIVA run # 3 : done
> STOP ABNORMAL END: S/R MDS_READ_FIELD
>  MDS_READ_FIELD: filename: ./ctrl_variables/adxx_atemp.0000000000 , ./ctrl_variables/adxx_atemp.0000000000.001.001.data
>  MDS_READ_FIELD: Files DO not exist
>  additional DIVA run # 4 : done
> STOP ABNORMAL END: S/R MDS_READ_FIELD
>  MDS_READ_FIELD: filename: ./ctrl_variables/adxx_atemp.0000000000 , ./ctrl_variables/adxx_atemp.0000000000.001.001.data
>  MDS_READ_FIELD: Files DO not exist

Quick status with not much time to follow through:
It is work in progress, but the changes to genmake2 ought to be foundational to make the rest work and should be maintained – once they work.
I do now see the errors that you are getting, a bit of a mystery. What is the case:
1/ ALL the TAF-generated _ad.f files are identical between original and modified genmake2 (which is good, and should be)
2/ oddly, where things differ is in the files that TAF does not see (e.g., the_model_main.F, ctrl_init_ctrlvar.f , and others) and where there is an ALLOW_ADJOINT_RUN CPP option. That option is not properly evaluated in these files, and which causes the errors due to missing code (e.g., all the adxx_ files are no longer initialized because ctrl_init_ctrlvar.f missed that code).
I don’t see yet how this can happen given my modifs (ALLOW_ADJOINT_RUN is set in AD_CONFIG.h, which has the correct #define).
The good news,

it is probably something dumb I did that may have an easy fix
all _ad.f files are still correct

From: Martin Losch @.>
Date: Thursday, March 9, 2023 at 12:02 PM
To: MITgcm/MITgcm @.>
Cc: @.*** @.>, Mention @.>
Subject: Re: [MITgcm/MITgcm] Modifs to genmake2 and adjoint_default to enable mixed .f .f90 with TAF (PR #709)
@heimbachhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fheimbach&data=05|01||5399727c9031452af14408db20c88067|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139817749546078|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=n8q1xLr1Mn4HdouvCKAVbhkkpg%2FD6PqA4JmA0EI8Xww%3D&reserved=0 I took the liberty to edit your posts (stripped the original messages that the emails replied to) to make it easier to follow this thread.
Thanks for the explanations, I now understand more of the changes. Is still work in progress, or is this already ready for review? If it is work in progress, we should add the corresponding label or make it a draft PR and I will hold off any further amateur attempts to make this work for me (it seems that make and Makefiles are my personal enemy).
Otherwise, I am not sure what to do here. The first thing I normally do is check if this PR breaks anything. I just used isomip, because it is usually fast to process (for taf), but the problem is the same for all other experiments. Also with lab_sea, the target adj_sed fails. When I fix this (see above just for adj_sed: move $(AD_F90FILES:.$FS90=_ad.$FS90) to the same line as $(AD_FILES:.$FS=_ad.$FS)), the lab_sea experiment compiles on a linux box (but not on my Mac, because the target adtaf is not made. I need to remove all_flowfiles.$FS to make it work there, and I am not sure why the -fixed flag is necessary, either. Let's leave the Mac issue for now), but it stops with this error (on linux):

MDS_READ_FIELD: filename: ./ctrl_variables/adxx_atemp.0000000000 , ./ctrl_variables/adxx_atemp.0000000000.001.001.data

MDS_READ_FIELD: Files DO not exist

additional DIVA run # 3 : done

STOP ABNORMAL END: S/R MDS_READ_FIELD

MDS_READ_FIELD: filename: ./ctrl_variables/adxx_atemp.0000000000 , ./ctrl_variables/adxx_atemp.0000000000.001.001.data

MDS_READ_FIELD: Files DO not exist

additional DIVA run # 4 : done

STOP ABNORMAL END: S/R MDS_READ_FIELD

MDS_READ_FIELD: filename: ./ctrl_variables/adxx_atemp.0000000000 , ./ctrl_variables/adxx_atemp.0000000000.001.001.data

MDS_READ_FIELD: Files DO not exist

—
Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMITgcm%2FMITgcm%2Fpull%2F709%23issuecomment-1462524086&data=05|01||5399727c9031452af14408db20c88067|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139817749546078|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=s7V14hX1rpDS6sP7qwe2aklcuq8brXe7bq%2F76ej4QnM%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABHJNDLWT3GTIUUSKKAIIGTW3ILK7ANCNFSM6AAAAAAVURWB4I&data=05|01||5399727c9031452af14408db20c88067|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|638139817749546078|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=eJwrQgTg1UcNTBDuPAj0fUogUMUrF%2B5%2Fr56uhW%2BzNRQ%3D&reserved=0.
You are receiving this because you were mentioned.Message ID: @.***>

I think I have a solution to the first issue (testreport experiments failing). Basically the same as for the target adj_sed: If the file list $(F90_PP_SRC_FILES) is empty (as for almost all experiments), then the line
	@\$(MAKE) -f \$(MAKEFILE) \$(F90_PP_SRC_FILES)

in genmake2 expands into
	make -f Makefile

without any files and this will then make the target mitgcmuv. I could push a modification to solve these issues and then you can check if it still works for your case. Let me know.
Also I think I know now why stuff is failing on the Mac: The filesystem does not differentiate between upper and lower case, so that the file suffixes $FS and $FS90 do not expand into f and f90, but into for and fr9, and I guess TAF does not understand that. That's why there is the -fixed option to make TAF understand that we are sending fixed-form source files. I guess this will be a fundamental problem for your approach. Or do you think we can convince TAF to accept these unusual suffixes? Fortran compilers do not seem to have a problem with that.

with the TAF 5.8.0 we can now specify both fixed and free format files with arbitrary suffixes like this
staf [options] -fixed fixed_format_file.for -free free_format_file.fr9

This solves the MacOS issue and it is no longer necessary concatenate all *.flow files into a new files with suffix .f.
@heimbach I can push a solution that contains all my previous suggestions, modified to work with TAF 5.8.0 for you to try. Let me know.

@heimbach please try my latest commit. You need to install the latest staf script for this (staf -get).

@mjlosch i'm going to try this... please stay tuned for some questions :-)

Hi all,
Thanks @mjlosch for pointing me to this PR!

I am also getting correct _ad.f files (after my fixes of adj_sed), but none of the AD testreport experiments pass. Either they do not compile (even after fixing adj_sed), or there are runtime issues with missing adxx_ files. I think that the problem is related to specifics of make.

It looks like the new genmake2 file never updates AD_CONFIG.h, leaving ALLOW_ADJOINT_RUN undefined. This is what causes adxx_* files to never write.

@IvanaEscobar I made the comment you are referring to on Mar 10, 2023. I have committed fixes since (e37d726). It would be great if you could try them.

I've updated genmake2 with @mjlosch 's committed fixes, and it seems to pass the one testreport I ran, not sure if the summary stating "Not enough lines..." matters.
Ran:
>>> cd verification/
>>> ./testreport -adm -ncad -t tutorial_global_oce_optim

Results:

My changes are in a forked branch of MITgcm, genmake2_hybrid
In that branch there are some potentially irrelevant contributions:
(1) genmake_local allowing TAF to extend source lines up to 132 columns (-e)
(2) adjoint_hybrid compile options file, which was suggested as a change adjoint_default in this PR. This allows for users to pass flags to TAF that aren't conventionally considered in genmake2
These modifications do not cause my targeted testreport to fail.

@IvanaEscobar It's great that it works for you, thanks for testing.
You'll find that "Not enough lines..." also happens with the master-branch. It just means that we did not turn on the adjMonitorFreq for this experiment (by choice).
About your branch genmake2_hybrid: I quickly looked at the differences with this branch and have these comments:

The changes in genmake2 cover the changes made in your version in genmake2_hybrid, don't they?
The -e option can already be specified in genmake_local by setting USE_EXTENDED_SRC='t', there is an example in verification/bottom_ctrl_5x5/build/genmake_local (or put this directly in the build-options file). Please try this with your specific configuration (i.e. not the tutorials or verification experiments) if this is already doing what you expect it to do.
adjoint_hybrid on your branch genmake2_hybrid is identical to the modified adjoint_default on this branch, right?
instead of adjoint_default, one can specify different AD-option files via genmake_local, and there is an example in verification/bottom_ctrl_5x5/build/genmake_local, and via the genmake2 -adoptfile=... flag. Extending this to testreport sounds like a good idea. Out of curiosity: is it possible to add these taf-specific flags to testreport -optfile=... file?
one can find adjoint_default in the documenation, maybe not very prominently. To me, this is the file (or a different version of this specified by -adjoptfile), where the taf-specific flags should go. The (undocumented) flags TAF_FORTRAN_VERS and (with this PR) TAF_COMMON_FLAGS (to be set in genmake_local) add some flexibility if one wants to test different things with the same adjoint options file.

I have this suggestion:
If @IvanaEscobar and @antnguyen13 can already use the genmake2 from this PR for their work (and if @heimbach agrees), I suggest to merge this PR as soon as possible and ask @IvanaEscobar to add the testreport contribution (point 4) in a separate PR. In fact, from what I see, such as PR would be completely independent of this PR and could be made even without this PR.

@mjlosch, I've checked on your points and address each one below:

@IvanaEscobar It's great that it works for you, thanks for testing. You'll find that "Not enough lines..." also happens with the master-branch. It just means that we did not turn on the adjMonitorFreq for this experiment (by choice).
About your branch genmake2_hybrid: I quickly looked at the differences with this branch and have these comments:

The changes in genmake2 cover the changes made in your version in genmake2_hybrid, don't they?

The changes in my branch are identical to this PR.

The -e option can already be specified in genmake_local by setting USE_EXTENDED_SRC='t', there is an example in verification/bottom_ctrl_5x5/build/genmake_local (or put this directly in the build-options file). Please try this with your specific configuration (i.e. not the tutorials or verification experiments) if this is already doing what you expect it to do.

Changing to USE_EXTENDED_SRC='t' works, which means we have two different ways of accomplishing passing -e?

adjoint_hybrid on your branch genmake2_hybrid is identical to the modified adjoint_default on this branch, right?

The adjoint_hybrid is fully defined by adjoint_default in this PR.

instead of adjoint_default, one can specify different AD-option files via genmake_local, and there is an example in verification/bottom_ctrl_5x5/build/genmake_local, and via the genmake2 -adoptfile=... flag. Extending this to testreport sounds like a good idea. Out of curiosity: is it possible to add these taf-specific flags to testreport -optfile=... file?

Leaving adoptfile as it's done in genmake2, as a unique option, allows for a user to ./testreport with a default optfile instead of iterating over which optfile works best followed by appending ad compile options to that file. I tried appending an optfile with a modified adjoint_default, which resulted in repitition of TAF-specific flags:
AD_TAF_FLAGS         = -f95 -e -i4 -r4 -intrinsic system,flush -reverse -l taf_ad.log -f95 -e -i4 -r4 -intrinsic system,flush -reverse -l taf_ad.log -toplevel 'the_main_loop' -input 'xx_obcsn_dummy xx_obcss_dummy xx_obcsw_dummy xx_obcse_dummy xx_genarr2d_dummy xx_genarr3d_dummy xx_gentim2d_dummy' -output 'fc' -toplevel 'the_main_loop' -input 'xx_obcsn_dummy xx_obcss_dummy xx_obcsw_dummy xx_obcse_dummy xx_genarr2d_dummy xx_genarr3d_dummy xx_gentim2d_dummy' -output 'fc'
FTL_TAMC_FLAGS       = -forward -ftlmark g_ -i4 -r4 -l tamc_ftl.log -forward -ftlmark g_ -i4 -r4 -l tamc_ftl.log -toplevel 'the_main_loop' -input 'xx_obcsn_dummy xx_obcss_dummy xx_obcsw_dummy xx_obcse_dummy xx_genarr2d_dummy xx_genarr3d_dummy xx_gentim2d_dummy' -output 'fc' -toplevel 'the_main_loop' -input 'xx_obcsn_dummy xx_obcss_dummy xx_obcsw_dummy xx_obcse_dummy xx_genarr2d_dummy xx_genarr3d_dummy xx_gentim2d_dummy' -output 'fc'
FTL_TAF_FLAGS        = -f95 -e -i4 -r4 -intrinsic system,flush -forward -l taf_ftl.log -f95 -e -i4 -r4 -intrinsic system,flush -forward -l taf_ftl.log -toplevel 'the_main_loop' -input 'xx_obcsn_dummy xx_obcss_dummy xx_obcsw_dummy xx_obcse_dummy xx_genarr2d_dummy xx_genarr3d_dummy xx_gentim2d_dummy' -output 'fc' -toplevel 'the_main_loop' -input 'xx_obcsn_dummy xx_obcss_dummy xx_obcsw_dummy xx_obcse_dummy xx_genarr2d_dummy xx_genarr3d_dummy xx_gentim2d_dummy' -output 'fc'

This seems to be genmake2 assuming adjoint_default TAF-specific flags before an -optfile is read.

one can find adjoint_default in the documenation, maybe not very prominently. To me, this is the file (or a different version of this specified by -adjoptfile), where the taf-specific flags should go. The (undocumented) flags TAF_FORTRAN_VERS and (with this PR) TAF_COMMON_FLAGS (to be set in genmake_local) add some flexibility if one wants to test different things with the same adjoint options file.
On my MacBook, genmake2_hybrid does not work.

Is this a .fr9 based issue? I ended up bonding the master branch's genmake2 in favor of one that generalized F90 file endings for the Mac environment. Works for forward models, based on the last set-ups I created in MacOS M2 environment. fwd genmake2 MacOS

I have this suggestion: If @IvanaEscobar and @antnguyen13 can already use the genmake2 from this PR for their work (and if @heimbach agrees), I suggest to merge this PR as soon as possible and ask @IvanaEscobar to add the testreport contribution (point 4) in a separate PR. In fact, from what I see, such as PR would be completely independent of this PR and could be made even without this PR.

This PR works for me, so let's hope it works for @antnguyen13 as well. I can set-up a separate PR for testreport

I did not follow the details, so just a side comment about a new testreport option:
I think it's nice when we can use testreport for multiple experiments (i.e., the default). If this new option allows to test an experiment that uses a mix of fixed and free-format code but does not work for current experiment that only use fixed-format, then it's probably better to leave this in the genmake_local.

@IvanaEscobar

10. On my MacBook, genmake2_hybrid does not work.

Is this a .fr9 based issue? I ended up abandoning master branch's genmake2 in favor of one that generalized F90 file endings for the Mac environment. Works for forward models, based on the last set-ups I created in MacOS M2 environment. fwd genmake2 MacOS

I hope that I am getting this right: in fwd genmake2 MacOS you modified this line 3258
      \$(TOOLSDIR)/f90mkdepend -fs $FS -fs90 $FS90 >> \$(MAKEFILE)

to include -fs $FS -fs90 $FS90. Is this something that we should add to this PR?
In this PR there are a lot of changes that relate to the TAF-AD generation, but there are no changes to forward-code generation. We could add this if it does not break anything (or again a different PR, your pick). I think genmake2_hybrid fails on my MacBook, because of missing pieces for the AD code, but I didn't check closely.

@IvanaEscobar

On my MacBook, genmake2_hybrid does not work.

Is this a .fr9 based issue? I ended up abandoning master branch's genmake2 in favor of one that generalized F90 file endings for the Mac environment. Works for forward models, based on the last set-ups I created in MacOS M2 environment. fwd genmake2 MacOS

I hope that I am getting this right: in fwd genmake2 MacOS you modified this line 3258
      \$(TOOLSDIR)/f90mkdepend -fs $FS -fs90 $FS90 >> \$(MAKEFILE)

to include -fs $FS -fs90 $FS90. Is this something that we should add to this PR?

Anything related to MacBook M1/2/3 could be addressed separately. This PR seems ready to go as is, unless @antnguyen13 finds issues with running.

@IvanaEscobar I removed the TAF_COMMON_FLAGS change. With this I think that the PR is ready unless you have anything to add. Please test this in your configuration.

Works, thanks for applying changes here.

I am going to merge the upstream/master branch to make the error in the html build go away.

That did not work, maybe @jahn has an idea what's going with build/doc_html? Can't be related to this PR (some numpy error).

See #845

Should be better now @mjlosch.

@jm-c This PR is good to go, in case you are looking for something to occupy yourself with. I modified the PR description a little.

There are a few considerations when compiling with F90 code that uses modules.

topological order for both forward and reverse compilation
linking the adjoint executable excluding any forward f90 object files

I don’t have the ability to contribute to this PR, but can meet via Zoom to walkthrough how I accomplish a successful TAF adjoint compile

I also tried to process with TAF some of the free-format src files from pkg/atm_phys but it did not work well with some module issues.

@jm-c can you provide a recipe to reproduce the problem? For F77 files, we had the same issue, but we fixed it somehow. It should be possible to do this for F90 files as well. The modules are a big differences, but that shouldn't be a show stopper.
@IvanaEscobar I would like to participate in a Zoom walkthrough, if you decide to do it.

6ae92ee addresses the issue of duplicate versions of the fwd routine in f90-code (in analogy to the solution for the f77 code).
To make it work for
https://github.com/christophernhill/gmao_mitgcm_couplng/tree/master/geos_bulkformulae/global_ocean.cs32x15
I had to

add TAF_FORTRAN_VERS=F90 to the genmake_local, because everything defaults to F77, in order to get rid off the -f77 flag
remove nobcs=4 from SIZE.h and SIZE_f90.h
modify a too long line in exf_geos_bulkf.F
...?

The "F90" files in this example do not contain any modules so it's not clear if this will work with modules.
Note that I have not yet made any attempt to fill the new file list NON_AD_F90_SRC_FILES, because I do understand the syntax to fill NON_AD_F77_SRC_FILES.

I have an update: for the geos_blkf ad built, I tried without "-ncad" and it works nicely (this was before today's latest changes from @mjlosch ). And I don't specify TAF_FORTRAN_VERS=F90 in genmake_local but instead set ALWAYS_USE_F90=1 (as done already in other exp. that uses Fortran-90 src code).
These findings suggest that this PR description could be improved (to mention what works and what does not yet).

And I don't specify TAF_FORTRAN_VERS=F90 in genmake_local but instead set ALWAYS_USE_F90=1

@jm-c that has the same effect, when ALWAYS_USE_F90=1, TAF_FORTRAN_VERS defaults to F90. Either way, the effect is that -f77 is replace by -f90 (something that I often do anyway, because it makes the AD-code easier to read).
I'll add something to the PR description.

I have an update: for the geos_blkf ad built, I tried without "-ncad" and it works nicely (this was before today's latest changes from @mjlosch ).

I tried that too, and yes, TAF generates something, but without the F90 files. They are not included in ad_input_code.f, so that I get:
TAF ERROR    Code of subroutine  helfsurface not seen and no flow information set,
TAF ERROR    assuming all arguments are input and output

and consequently the link step fails because these routine are missing.
This makes sense, as TAF needs to know if the files are fixed or free form; by default TAF decides based on the file extension (.f or .f90), but with the flags -fixed, -free, we can now tell it what format-files follow independently of the file extension. For the case without -ncad, the modifications in the PR are incomplete (we'd need something like an ad_input_code.f90 and taf_output_ad.f90 or similar that collects the f90-files). We need to decide if we want to allow this case at all, because it means extra work to extend a legacy option.

OK, the test that I reported earlier were not right, because I was using "-mods code_geos_ad  geos_blkf" instead of "-mods geos_blkf code_geos_ad" (and USE_GEOS_BULKF" is undef in code_geos_ad/EXF_OPTIONS.h` !!!). So things are clear now, and I've checked that with @mjlosch latest changes I can generate the executable when using "-ncad".

@mjlosch your new version of genmake2 seems more complicated than the previous one, meaning keeping the 2 "$TMP.adFIles" make this processing simpler (and may be even faster). Going to push an alternative version more like the previous one, but getting all the src files (both ".f" and ".f90") from "_ad_diff.list" only.
Also, seems more consistent to fill the "NON_AD_F90_SRC_FILES" also with OpenAD since it's part of "AD_OBJ_FILES_S2".

So I went back to 2 "$TMP" files, one for "*.f" src files and one for "*.f90" files. I use a very simple "grep" command, hopefully it works on all systems.
@mjlosch there is something I am not getting regarding the alternative suffix "*.for": The suffix get changed (around lines 2788-2791) from ".f" to ".for", in "$TMP.adSrcFiles", but to get the list of NON_AD_F77_SRC_FILES, line 3015:

                         isAD=`egrep ^$basename.f'[  ]*' $TMP.adSrcFiles`

we search for a ".f" file. Not clear to me how this works. And also this it not what you added for the f90 equivalent where we grep for a ".$FS90" suffix (now line 3053).
I think it would be simpler (and I could push this little change) if the "$TMP.adSrcFiles" was kept unchanged (i.e., with original suffix) and we do the suffix substitution only when it's put into genmake2 local var AD_FILES (line 3435).

@mjlosch I will push my simplifications and will ask you to check again that this works (I am not in a good position to test that since it's related to alternative suffix ".for" and ".fr9" that I don't use).

few things:

Regarding OpenAD, I suggest not to change anything (this means reverting the changes to line 3841 in genmake2 master branch, now line 3877) since it's on its way out. Once NON_AD_F90_SRC_FILES is removed from the OpenAD AD_OBJ_FILES_S2 list we could skip the part that generates "$TMP.nonADF90srclist" if using OpenAD (as you did earlier).
Regarding Tapenade, could go the same route and leave further changes for free-format files to an other PR. So it's probably OK as it is now, except that I need to check for cleaning of "$TMP" files.
Regarding @mjlosch suggestions related to AD_FILES (and AD_F90FILES) into the Makefile, I will follow the first suggestion but keep the same content for the genmake2 local var AD_FILES (i.e, not go for the second suggestion) since I like the nice matching between the local var and the list inside the Makefile (+ it could be re-used elsewhere in genmake2 if needed).

Will make these changes and push an updated version.

Other comments:

I still don't like the idea to use the same current adjoint_sed also for free-format f90 files. It's also not used consistently (e.g., in "adj_sed" but not in "ftl_sed"). From my side, it looks easier to skip this step for TAF generated f90 files.
I don't know what the ad_input_code.$FS90 part is used for (lines 3482-3489) since, for now, the mix of f77 and f90 is only supported with "-ncad". It would be cleaner to remove this bits that cannot be used.

I think I addressed @jm-c latest comments. I do have a version that (almost, one dependency missing) works without -ncad, but only if there is at least one free-format file (does not work with empty AD_F90FILES list). I don't think that we need this now, so I won't invest more time in this direction, unless someone wants to see this.

I'll happily revert 383302d if this is found to be unnecessary (except for the extra -fixed options in ftl-targets).

@mjlosch Thanks for the addition. I will take a look, but even if this does not always work without "-ncad" it's good that you manged to have it to work (some time) like this so that the pieces around ad_input_code.$FS90 have some uses (and my second "Other comment" from yesterday can be discarded).

@mjlosch I have few comments:

The changes in tutorial_global_oce_optim are valuable (got this tested) but it's going to slow down the merging of this PR (need to check more things).

Without "-ncad", it's still not very clear why we need this new target "ad_taf_output_f90.$FS90", but willing to trust you and you added comments for that (lines 3532-3533) so it's fine for now (could revisit this later in an other PR). I would remove the next 2 other lines of comments (lines 3534-3535), since it does not add much (about something that is not there anyway).

Without "-ncad", a detail regarding file names: I would prefer to change/rename:

ad_input_code_f90.f90   ->  ad_inpF90_code.f90
ad_taf_output_f90.f90    ->  ad_taf_outpF90.f90

this would also change:

ad_input_code_f90_ad.f90   ->  ad_inpF90_code_ad.f90

would be a little bit shorter and, from my side, a little bit more clear: 2 type of input (input & inpF90), 2 type of output (output & outpF90 ), would avoid a piling up to suffix in "ad_input_code_f90_ad.f90" plus may be a little bit more convenient to list the relevant ones from verification in all build dirs.

with "-ncad", I am getting this error when running genmake2:

../../../tools/genmake2: line 3483: test: too many arguments

when there is more than 1 file in "$AD_F90FILES".

I did not try, but looks like some changes have been added for Tang.Linear but likely missing some. Could leave this to an other PR (just mention it in PR description) but, in this case, would be cleaner to not change this part at all.

I think my latest commit should fix the genmake2 error when there is more than 1 file in "$AD_F90FILES" (point 4 in the list above).

the modified verification test was just a suggestion (forgot to post a comment), and we can revert it and do it in a separate dedicated PR (to be decided)
need the new target to get the dependencies right for "make"
I am happy to change the names.
thanks for fixing
I'll revert the NON_AD_F90_SRC_FILES to F90_SRC_FILES, they are wrong. The -fixed in the ftl-target was there before implicitly in TAF_EXTRA, but this gets removed in this PR (l1830 in master)

I tried using the updated PR on @heimbach 's original test problem using -ncad with GNU compilation on a Linux machine. A TAF error regarding the f90 code comes up:
TAF ERROR    Code of subroutine  helfsurface not seen and no flow information set,

Looking into why, I found that AD_F90FILES is empty.

Didn't realize *.F90list was deprecated. After appending *.F90list to *.list, the error disappears and I am able to get a working mitgcmuv_ad executable!

I am glad you were able to solve this. This change was only poorly documented in the commit messages of 0906772, which resolved an earlier comment. Is there a way to update the original test? For now I'll add a comment in the PR description at the top.

The code modifications I used aren't contained in this PR. I submitted a PR in the GMAO MITgcm coupling repository with changes needed to get this genmake2 to work.

I've just added a 3rd bullet point in this PR description, in section "Other Information". @mjlosch I will review your latest changes and see how far we are before merging this PR.

Hi all, the https://github.com/christophernhill/gmao_mitgcm_couplng repository is deprecated.
It has been replaced by: https://github.com/geos-mitgcm-ecco/gmao_mitgcm_coupling
Ivana and Martin, would it be possible to move this discussion to the new repo?
Let me know if either or both of you (or anyone else who want to work with this repo)
wants write access to https://github.com/geos-mitgcm-ecco/gmao_mitgcm_coupling
Thanks, Dimitris

On Aug 6, 2024, at 9:10 AM, Ivana Escobar @.***> wrote:
The code modifications I used aren't contained in this PR. I submitted a PR in the GMAO MITgcm coupling repository https://github.com/christophernhill/gmao_mitgcm_couplng/pull/4 with changes needed to get this genmake2 to work.
—
Reply to this email directly, view it on GitHub https://github.com/MITgcm/MITgcm/pull/709#issuecomment-2271652341, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABH7CSLZVO62NYZ23MAHLOLZQDYNTAVCNFSM6AAAAAAVURWB4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZRGY2TEMZUGE.
You are receiving this because you are subscribed to this thread.

@menemenlis
Transferred PR to active repo

Transferred PR to active repo

@IvanaEscobar can you modify the PR description (top of the page) to explain how to run this now? (or provide the recipe here and I modify the PR description).

@IvanaEscobar I could take care of updating this PR description regarding the switch from:
https://github.com/christophernhill/gmao_mitgcm_couplng
to:
https://github.com/geos-mitgcm-ecco/gmao_mitgcm_coupling
Just let me know.

@IvanaEscobar I could take care of updating this PR description regarding the switch from: https://github.com/christophernhill/gmao_mitgcm_couplng to: https://github.com/geos-mitgcm-ecco/gmao_mitgcm_coupling Just let me know.

Couldn't make changes to this PR, so I updated a text file in the GMAO coupling repo to include a recipe for compilation and updated the repo address to https://github.com/geos-mitgcm-ecco/gmao_mitgcm_coupling

@IvanaEscobar Thanks, it's great. I will update this PR description.

I've updated the PR description. I mention the old repos there since it's still relevant for the history of discussion and also mentioned @IvanaEscobar PR there (it has been merged recently). Please let me know if I need to change something there.

Thanks @jm-c

OK, from my side, this PR is ready to be merged. I know that @mjlosch was fine with that too after his last set of changes. @IvanaEscobar or @heimbach could you confirm from your side that it's good to merge too ?

Works on our end too!

Going to merge this PR later today.


Can I use this plugin while using openai's api?

I have not applied for the official plug-in now, and I want to use this plug-in function when calling the api.

Plugins are only available through ChatGPT interface, not through the API (and not currently on their roadmap) [1]


Serde rename convention for arbitrary delimiters.

I'm currently writing some code that is meant to deserialize a field into a Rust enum. The scheme of the values in this field are period-delimited values. Here are some examples:

"item.created"
"item.deleted"
"user.profile.created"
"user.profile.deleted"

It would be cool if I could still use rename_all instead of individual renames. Maybe something like this?
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "lowercase", separator = ".")]
pub enum Event {
    ItemCreated,
    ItemDeleted,
    UserProfileCreated,
    UserProfileDeleted,
}

A more general version might be
fn lowercase_with_dot(parts: &[&str]) -> String {
    parts.map(|p| p.to_lowercase()).join(".")
}

#[derive(Serialize, Deserialize)]
#[serde(rename_all_with = lowercase_with_dot)]
pub enum Event {
    ItemCreated,
    ItemDeleted,
    UserProfileCreated,
    UserProfileDeleted,
}

I'd prefer this over the original proposal, because it covers every rename conventions that might come up, without having to change serde.


pkg_resources.DistributionNotFound: The 'gcloud' distribution was not found and is required by the application

why this code return error?use python 2.7

gcloud expects to be installed as a top-level package in the site-packages directory of your Python or virtualenv.  You shouldn't be importing it from lib.gcloud.


HDMI pass-thru doesn't work if lgx2userspace is not started

Since I used lgx2userspace, HDMI pass-thru only works if I start lgx2userspace once. I can even close it again, it still works.
That might be an issue of not de-initialising (#18), just a guess, tho^^
I still didn't try Windows again...

Unplug LGX2 USB cable (For me, the unplugged time was always over night)
Plug LGX2 USB cable
Don't start lgx2userspace
Check if pass-thru works
Start lgx2userspace
Pass-thru should work now

Hi,
Yeah the HDMI pass-thru will remain active the entire time after you have started the lgx2userspace program.
The device remains in a state and there is no tear down that happens at all currently.
I'm going to close this, but if you need to add anything feel free to open it back up! Cheers!


Test against Python 3.7 and support pytest 4.x

This PR adds support for Python 3.7 and will be updated to address #100, #110, #92 , #125

For reference, setting COVERALLS_REPO_TOKEN is required to use coveralls python client or to submit coverage results using curl. Failing to do so reports Couldn't find a repository matching this job.
See https://github.com/lemurheavy/coveralls-public/issues/487 and https://github.com/z4r/python-coveralls#configuration

@bmoscon @eeaston @javefang  This is now ready for final review.
It addresses issues #125 #122 #110 and #100
After getting your feedback, I will also:

[ ] update CHANGES.md
[ ] update the install_all target in the Makefile so that python 3.7 is also installed in Vagrantfile

Once, this is done it would be great to cut a release and have all the wheels uploaded to pypi. This will all me to fix testing associated with cmake-python-distributions, ninja-python-distributions and scikit-build
Cc: @thewtex

@eeaston @javefang looks good to me, but given the size and scope of the change I'd like to have another review before merging

Et voila. The last two updates have been pushed. Now looking forward final reviews.
In the mean time and waiting new official wheels are released, temporary ones are available at https://github.com/jcfr/pytest-plugins/releases/tag/v1.7.0.dev15
They can be installed using:
pip install pytest-virtualenv -f https://github.com/jcfr/pytest-plugins/releases/tag/v1.7.0.dev15 --pre

LGTM, thanks for the contribution!
@eeaston , I tested the branch against pytest==3.1.0 which is also a pass.

When can have this merged as now I went into conflicts caused by:
pytest-verbose-parametrize 1.5.1 has requirement pytest<4.0.0, but you have pytest 4.3.1.

@ssbarnea As a temporary workaround, you could install corresponding development wheels. See https://github.com/manahl/pytest-plugins/pull/130#issuecomment-471149935

@jcfr Thanks for the hint but I will have to pass. I would not be able to do this even if the pre-release would be on posted on pypi as we have a policy of not using unreleased dependencies (not a real surproise).
For the moment I added a <4.0.0 on pytest with a comment blaming pytest-verbose-parametrize as being the reason.

@eeaston this ok to merge? I'll probably squash and merge it soon unless you want to review/comment

For convenience, here is a squashed version of that same topic:

Hi guys apologies for the tardy replies.
@jcfr  - amazing PR, many thanks for the hard work, I'll start going through it.

LGTM, many thanks! I'll approve the squashed PR #134

I'll approve the squashed PR #134

Closing this one.


fix. when tokio/console is added, tracing::macro! in the same scope o…

Fix when tokio/console is added, tracing::macro! in the same scope of a suspension point does not compile.

bors merge

bors merge


Fix wrong deployType for BMS Copy

@dennis-behm  Can you integrate this small change quickly?

I am merging this one to development; we will add it back in later.


Proposal: DAG-based (dependency) model of workflow execution

This issue is to document a proposal and facilitate discussion on having an alternative model of workflow execution using a bottoms-up DAG dependency based approach vs. our current model of a top-down steps-based approach. We would still continue to support the steps-based execution model, but provide the option to execute workflows using the DAG-based model.
The idea behind a dependency approach is similar in concept to a Makefile, where you have a target, that target has dependencies, which in turn have their own dependencies. The execution engine would figure out a resolution order in order to reach the desired target, and run the nodes in the most optimal way possible (essentially when all of a target's dependencies have been satisfied).
Consider the following run-of-the-mill diamond workflow pattern which fans out and fans back in:
     ONE
    /   \
  TWO  THREE
    \   /
    FOUR

The following is how it might be represented in our workflow yaml by declaring dependencies between targets (aka nodes):
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: diamond-dag-
spec:
  target: four
  templates:
  - name: whalesay
    inputs:
      parameters:
      - name: message
    container:
      image: docker/whalesay:latest
      command: [cowsay]
      args: ["{{inputs.parameters.message}}"]

  targets:
  - name: one
    template: whalesay
    arguments:
      parameters: [{name: message, value: one}]
  - name: two
    dependencies:
    - one
    template: whalesay
    arguments:
      parameters: [{name: message, value: two}]
  - name: three
    dependencies:
    - one
    template: whalesay
    arguments:
      parameters: [{name: message, value: three}]
  - name: four
    dependencies:
    - two
    - three
    template: whalesay
    arguments:
      parameters: [{name: message, value: four}]

This is how we are able to achieve the same functionality today:
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: diamond-steps-
spec:
  entrypoint: diamond
  templates:
  - name: whalesay
    inputs:
      parameters:
      - name: message
    container:
      image: docker/whalesay:latest
      command: [cowsay]
      args: ["{{inputs.parameters.message}}"]

  - name: diamond
    steps:
    - - name: one
        template: whalesay
        arguments:
          parameters: [{name: message, value: one}]
    - - name: two
        template: whalesay
        arguments:
          parameters: [{name: message, value: two}]
      - name: three
        template: whalesay
        arguments:
          parameters: [{name: message, value: three}]
    - - name: four
        template: whalesay
        arguments:
          parameters: [{name: message, value: four}]

Below are some of the key benefits of the DAG model which I will attempt to outline:

Highly optimized target execution. Execution of the nodes in the DAG will be highly optimized because we would be able to execute nodes as soon as its dependencies have been satisfied.

Flexibility in control flow. With today's steps based model, we can approximate a DAG, but never truly achieve the same level of functionality.

However, there are some key drawbacks for DAG workflows

Difficult mental model. The bottoms up approach of building dependencies can be somewhat of an unnatural way to think of some workflows. The top down approach would feel more natural for workflows which are more closer to a script than a DAG. It may also be harder to understand and express branching in a DAG.

Dynamic workflows would be difficult to achieve. Consider the coinflip-recursive.yaml example, which is an example of a dynamic workflow which recursively runs a template until it reaches a desired outcome. With DAGs, targets are typically pre-defined at submission and dynamically injecting new targets on the fly, set up relationships between them for something like a recursive workflow would be difficult. I also do not believe we would want to allow something like circular dependency of targets.

CLI rendering. Our steps-based workflow is easily represented as a tree with a single root. Each node has a predictable pattern (every template is either a leaf, or a node which fans out and fans back in). Due  of this, we are able to render the tree quite easily in the CLI. But since DAGs are completely free-form in their relationships between nodes, I do not think it will be feasible to render a free-form DAG from the CLI.

In the end, my belief is that users may still end up preferring a steps-based approximation because of its simpler mental model.

Here is a simple, concrete example of a DAG, which does not have a good approximation using a steps-based execution (node dependencies goes downward):
    A   B
   / \ /
  C   D

The closest approximation of this using steps would be something like:
    *
   / \
  A   B
   \ /
    *
   / \
  C   D
   \ /
    *

As you can see, C's execution gets delayed since it waits on B unnecessarily.

After discussion the decision is to make dag a new template type, which defines a list of tasks from within that overall encompassing step. Example below:
# The following workflow executes a multi-root workflow
#
#   A   B
#  / \ /
# C   D
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: diamond-dag-
spec:
  entrypoint: multiroot
  templates:
  - name: echo
    inputs:
      parameters:
      - name: message
    container:
      image: alpine:3.7
      command: [echo, "{{inputs.parameters.message}}"]
  - name: multiroot
    dag:
      tasks:
      - name: A
        template: echo
        arguments:
          parameters: [{name: message, value: A}]
      - name: B
        dependencies:
        template: echo
        arguments:
          parameters: [{name: message, value: B}]
      - name: C
        dependencies: [A]
        template: echo
        arguments:
          parameters: [{name: message, value: C}]
      - name: D
        dependencies: [A, B]
        template: echo
        arguments:
          parameters: [{name: message, value: D}]

What do you think about actor model?

@bhack i'm unfamiliar with that concept. Is there a framework that uses this pattern that you could point to so I can understand further?

Take a look to this specific Go presentation

Another quite popular Go project is https://github.com/AsynkronIT/protoactor-go

Also for scientific workflows. There is also a related video

Thanks for the links -- I watched the video. Based on what I've learned, actor models seem more of a programming paradigm requiring a framework/SDK to handle the plumbing and messaging between nodes and their actors. This issue is more about how to support a traditional dependency based DAG definition within an container-based workflow. It's definitely an interesting model which we may draw concepts from in the future, but seems be orthogonal to the issue at hand, which is defining a DAG in k8s yaml.

Yes it is a little bit orthogonal to this issue cause the message passing component of the actor model pattern doesn't exist on the shelf in K8 core. But could be an interesting workflow execution paradigm cause could include explicit data exchange definition in the workflow execution model.

@jlewi FYI

This is great!
I think this will make it easier to replace Airflow with Argo since Airflow offers the ability to do arbitrary DAGS.
I think in some ways specifying the dependencies is much easier than trying to manually flatten the graph into the current representation.
My general expectation though is that for really complicated graphs you'd want to use higher level tooling to generate them and not write them by hand.

Yes, we also think higher level tooling is needed for complex workflows.
We're also experimenting with invoking Airflow workflows from Argo workflows. This would let users mix and match existing Airflow operators/DAGs with container based workflows and leverage the huge catalog of existing container images.

DAG support is now in master and will be available as part of the upcoming v2.1 release


[✨] Display a warning for not using a unique key while mapping inside in JSX

Is your feature request related to a problem?
According to Qwik docs, a unique key should be given to the parent element within a map() statement.
But, while mappping inside JSX, we don't see any warnings in console if a key is not given, and this could cause problems which the developer might not be able to debug instantly.
Describe the solution you'd like
I think it would be nice if we get some warnings in console so that the developer knows if they missed a place where the key was not given.
Describe alternatives you've considered
I'm not sure about alternatives, but I think showing an error or warning in console would be pretty straight forward.
Additional context
No response

I'm closing this one because now we have the warning.
Thanks @sharifrezvani for your report


Libraries issue Android

731f4f Fixed  gemoetry mapping
8379fd8 pkg rpm: add version information to free-nightly

@emeraldservices you could add some text that describes your issue, that might help others to understand what is going on. Closing until then


Allow escaped quote literals in the SyntaxScopeMap

As raised by @Ben3eeE in https://github.com/atom/language-json/pull/68#issuecomment-492440942, we are currently unable to apply decorative syntax scopes to " literals for TreeSitter grammars, which is forcing us to break user expectations around how syntax themes apply to strings.
In this PR, I add some special handling for escaped " literals to the SyntaxScopeMap. Internally, the SyntaxScopeMap uses the postcss-selector-parser library to parse any scope it is provided. This library allows " literals inside of selectors, but they need to be escaped in the selector. So a valid post CSS selector looks like this:
string > "\""

Unfortunately, these selectors are themselves expressed as strings within CSON or JSON files in our grammars. That means the escaping gets a bit intense, and looks like this:
'string > "\\""': 'puncuation.quote.double.whatever'

Notice that I include 2 backslashes when escaping the quote so that they end up getting passed through to the parser as a single \. If I only include one backslash it ends up looking like an escaped quote at the interpretation level of CSON rather than the level of the selector language. 😓
Anyway, once properly escaped, the quote literal is handled by the parser, but the value it spits out continues to include the escaping backslash. In order to match the text of the anonymous nodes returned by TreeSitter, we need to remove the escaping after trimming off the outer quotes.
@Ben3eeE hopefully this gives you what you need to style quotes properly in various grammars.
cc @50Wliu @maxbrunsfeld

That CSS syntax is pretty sweet. Curious this and the state machine, but it's probably low on the list of priorities right now for integration into Atom.

Flaky test in GitHub package that's definitely unrelated. Merging.

Flaky test in GitHub package that's definitely unrelated. Merging.


Saving Dataframe to HDFS

While Saving data frame to HDFS using  dataFrame.write.format("com.databricks.spark.csv").save("/data/airsell_segment/landing/pss_kafka")  data is getting written to the hdfs folder but under the double quotes
e.g
"Ashu,Singh,84,XXX,Yrert"     (single row)
Ashu,Singh,84,XXX,Yrert      (i want)
but i want it to be double quote free please do let me know the proper way to do that.

Have you tried quote and quoteMode options as described in README.md?

I have tried like this
dataFrame.write.format("com.databricks.spark.csv").option("quoteMode","NONE").option("escape", "\").save("/data/airsell_segment/landing/pss_kafka") but now the records are coming
160906,212115,7174EAAECEAAD04,5MRA,5MRA,JA,1S,,,01,03,1,AZ,1268,170507\


[BEAM-12685] Allow managed thread count in AvroIO

This change supplies a config to AvroIO called getReaderThreadCount which allows a user to control the number of reader threads used by the ReadAllViaFileBasedSource avro file reads.
Without the change, any defined groups are removed during ReShuffle and the design can easily cause OOM errors to occur.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[x] Choose reviewer(s) and mention them in a comment (R: @username).
[x] Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

R: @pabloem

FYI, the streaming test is failing because of https://issues.apache.org/jira/browse/BEAM-12699

hmmm I think we usually try to avoid giving this kind of knob to users, because it's easy for these parameters to become obsolete, and hardcoded in legacy pipelines.. I'm wondering - do you think there's another way of accomplishing this?
In Dataflow we have GroupIntoBatches.withAutoSharding, which provides a sharding parameter that scales automatically with the number of workers. Perhaps this would help?

We could remove the ReShuffle entirely?  There isn't a clear reason to change whichever setting were set upstream or a reason why the reshuffle adds much value.
More than anything it causes a forced commit if you are reading Pub/Sub and removes a grouping which was set on purpose?  It seems like there isn't a strong reason to shuffle here

Reshuffles were typically inserted since it might be well known that the upstream transforms had limited parallelism and without it you would be limited to the upstream parallelism.

Reshuffles were typically inserted since it might be well known that the upstream transforms had limited parallelism and without it you would be limited to the upstream parallelism.

My issue is that the reshuffle is having the inverse effect, since we force full parallelism at this stage its very easy to cause OOM crash loops when reading many files at once.
At least in the Dataflow runner, it does not appear it knows how to properly scale down the number of threads when this sort of issue occurs to avoid the issue.  Removing the reshuffle at least allows for someone to force their desired behavior when they know what they want rather than force a single aggressive strategy.
I suppose the feature flag could control if the reshuffle occurs at all to maintain the current behavior by default and allow the user to manage the parallelism where needed

Run Java PreCommit

Run Java PreCommit

examples broken on master: https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/

Run Java PreCommit

Run Java PreCommit

the change lgtm. I'll try to get a passing run of precommit to have some confidence, and merge.

added a quick fix to the javadoc

Run Java PreCommit


Update json builder to generate AAPB formatted json transcripts

Update: transcript-editor/app/views/transcripts/show.json.jbuilder
Sadie can provide an example AAPB formatted json transcript, so we can try to align the output from this system more with that. It would be nice not to have to run further scripts on the output before these json files get used by AAPB.

@sroosa - so, the issue with the PR that I tried to merge is that the front end view relies on transcript-editor/app/views/transcripts/show.json.jbuilder in the original format. The code in the PR overwrote that entirely which caused the view and some JS to fail. Would it be OK to just tack on the 'parts' formatting to the end of transcript-editor/app/views/transcripts/show.json.jbuilder? Or should we render a whole new endpoint entirely where we have a totally clean AAPB formatted json transcript? Putting this here more for remembering what the issue is for Monday!

NOTE: We decided to create a new appb transcript endpoint.

Closed by #79


subctl diagnose all should validate all the use-cases even when there is an error

What happened:
While running subctl diagnose all, even if an error is seen in one of them (like connections) it should continue to validate the remaining stuff and should not exit.

Fixed in the following PR - https://github.com/submariner-io/submariner-operator/commit/8eb5ecbea1d541037145ae54ba51c9711d194ecd


[V3-V4] migrating our keyframes, inline styles are now broken.

We are updating an old library that use a lot of keyframes from styled-components v3 to styled-components v4.

We use a makeKeyframe function that give back the keyframe classame.
We use that classname to create an inline style.

It use to look like this:
const keyframeName = makeKeyframe(this.makeAnimation, { distance, rotation, perspective, amplification }, props.keyframes);
styles.animation = ` ${keyframeName} ${duration} ${timingFunction} ${delay} ${iterations} ${direction} ${fillMode} ${playState}`;
if (TYPE_ROTATE === this.makeAnimation.type) {
    styles.backfaceVisibility = backfaceVisibility;
}

// later
this.setState({
   styles,
});

// on render
return (
  <span
    className={mapToCssModules(cn({ 'd-inline-block': inline }, className), cssModule)}
    styles={this.state.styles}
  >
    {children}
  </span>
);

After upgrading to v4, we had the following error:
  Uncaught Error: It seems you are interpolating a keyframe declaration (bGJZDZ) into an untagged string. This was supported in styled-components v3, but is not longer supported in v4 as keyframes are now injected on-demand. Please wrap your string in the css\`\` helper (see https://www.styled-components.com/docs/api#css), which ensures the styles are injected correctly.

We switched in in v4 to :
let styles = css`
    animation: ${keyframeName} ${duration} ${timingFunction} ${delay} ${iterations} ${direction} ${fillMode} ${playState};
  `;

  if (TYPE_ROTATE === this.makeAnimation.type) {
    styles = css`
      ${styles}
      backface-visibility: ${backfaceVisibility};
    `;
  }
// later
this.setState({
   styles,
});

// on render
return (
  <span
    className={mapToCssModules(cn({ 'd-inline-block': inline }, className), cssModule)}
    css={this.state.styles}
  >
    {children}
  </span>
);

This also doesn't work.
.hDnAJR {
    -webkit-undefinedanimation: bGJZDZ 500ms ease 0ms 1 normal none running;
    -ms-flex-undefinedanimation: bGJZDZ 500ms ease 0ms 1 normal none running;
    undefinedanimation: bGJZDZ 500ms ease 0ms 1 normal none running;
    -webkit-undefined: ;
    -ms-flex-undefined: ;
    undefined: ;
}

We did use the css helper, but now the injected keyframes in the pages cannot be used without react is that correct ?
Or is there a way to fix this?

Solved by callin css on render:
return (
  <span
    className={mapToCssModules(cn({ 'd-inline-block': inline }, className), cssModule)}
    css={css`${this.state.styles}`}
  >
    {children}
  </span>
);

const style = {
  rollin: css`
    border: 1px solid red;
    animation-name: ${({ isAnimated }) =>
      isAnimated ? `${zoomIn}` : `${scaleOut}`};
  `,
  zoom: css`
    border: 1px solid red;
  `,
};


Arch linux - ascii codec can't decode byte 0xe2...

Codebase
build-in
Host OS
Arch
Docker info output
Containers: 2
 Running: 0
 Paused: 0
 Stopped: 2
Images: 230
Server Version: 17.06.0-ce
Storage Driver: overlay2
 Backing Filesystem: extfs
 Supports d_type: true
 Native Overlay Diff: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Plugins:
 Volume: local
 Network: bridge host macvlan null overlay
 Log: awslogs fluentd gcplogs gelf journald json-file logentries splunk syslog
Swarm: inactive
Runtimes: runc
Default Runtime: runc
Init Binary: docker-init
containerd version: cfb82a876ecc11b5ca0977d1733adbe58599088a
runc version: 2d41c047c83e09a6d61d464906feb2a2f3c52aa4
init version: 949e6fa
Security Options:
 seccomp
  Profile: default
Kernel Version: 4.12.8-2-ARCH
Operating System: Arch Linux
OSType: linux
Architecture: x86_64
CPUs: 8
Total Memory: 7.663GiB
Name: arch
ID: 4DRX:LG2Y:A4TI:R3AK:3JXP:PQ2Y:XXKQ:ZJ5O:YQCK:75UF:GLD5:A33O
Docker Root Dir: /var/lib/docker
Debug Mode (client): false
Debug Mode (server): false
Registry: https://index.docker.io/v1/
Experimental: false
Insecure Registries:
 <IP_ADDRESS>/8
Live Restore Enabled: false

Docker compose file
Exactly what's in the repo, no changes.

When I run
docker-compose up -d

I get
Traceback (most recent call last):
  File "/usr/bin/docker-compose", line 11, in <module>
    sys.exit(main())
  File "/usr/lib/python3.6/site-packages/compose/cli/main.py", line 68, in main
    command()
  File "/usr/lib/python3.6/site-packages/compose/cli/main.py", line 118, in perform_command
    project = project_from_options('.', options)
  File "/usr/lib/python3.6/site-packages/compose/cli/command.py", line 37, in project_from_options
    override_dir=options.get('--project-directory'),
  File "/usr/lib/python3.6/site-packages/compose/cli/command.py", line 87, in get_project
    config_details = config.find(project_dir, config_path, environment, override_dir)
  File "/usr/lib/python3.6/site-packages/compose/config/config.py", line 274, in find
    [ConfigFile.from_filename(f) for f in filenames],
  File "/usr/lib/python3.6/site-packages/compose/config/config.py", line 274, in <listcomp>
    [ConfigFile.from_filename(f) for f in filenames],
  File "/usr/lib/python3.6/site-packages/compose/config/config.py", line 174, in from_filename
    return cls(filename, load_yaml(filename))
  File "/usr/lib/python3.6/site-packages/compose/config/config.py", line 1303, in load_yaml
    return yaml.safe_load(fh)
  File "/usr/lib/python3.6/site-packages/yaml/__init__.py", line 94, in safe_load
    return load(stream, SafeLoader)
  File "/usr/lib/python3.6/site-packages/yaml/__init__.py", line 70, in load
    loader = Loader(stream)
  File "/usr/lib/python3.6/site-packages/yaml/loader.py", line 24, in __init__
    Reader.__init__(self, stream)
  File "/usr/lib/python3.6/site-packages/yaml/reader.py", line 85, in __init__
    self.determine_encoding()
  File "/usr/lib/python3.6/site-packages/yaml/reader.py", line 124, in determine_encoding
    self.update_raw()
  File "/usr/lib/python3.6/site-packages/yaml/reader.py", line 178, in update_raw
    data = self.stream.read(size)
  File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 436: ordinal not in range(128)

Anyone have any ideas?

It's a docker-compose issue, see https://github.com/docker/compose/issues


Input univariates in the Sumcheck Relations

The sum check code should panic if the  Input univariates in the sumcheck relations are not linear polynomials.

As we discussed in scrum, you are referring to something that would happen inside of a relation's add_edge_contribution function, but the performance of these functions is critical, so we cannot tolerate any additional overhead. The more particular concern is to prevent https://github.com/AztecProtocol/barretenberg/blob/2773c5f10bbae5dbd5bd6ff3a830d5513def8b8d/cpp/src/aztec/honk/sumcheck/relations/relation.test.cpp#L105 from being defined in such a way that the array elements are not in fact 'extended edges', meaning that the univariates are not of degree < 2 as polynomials. It therefore makes sense to focus on adding functionality to create univariates of a desired form, either within the test or within the univariate classes. That said, we can discuss other solutions that don't affect performance of the sumcheck prover.


FastCGI path doesn't seem to be very restrictive

1. What version of Caddy are you running (caddy -version)?
Caddy 0.8.2
2. What are you trying to do?
Make it so that FastCGI will only be used for index.php: All other php files should print text.
3. What is your entire Caddyfile?
img.sharo.se {
        tls<EMAIL_ADDRESS>        root /opt/img.sharo.se
        fastcgi /index.php /var/run/php5-fpm.sock php
        log stderr
        errors stderr
}

4.
Running via upstart script as follows:
description "Caddy Server"
author "ShaRose"

start on runlevel [2345]
stop on runlevel [016]

setuid caddy
setgid caddy
respawn
respawn limit 10 5
limit nofile<PHONE_NUMBER>00
env HOME=/opt/caddy
script
        cd /opt/caddy
        exec /opt/caddy/caddy -agree -cpu 75% -conf=/opt/caddy/Caddyfile -pidfile=/opt/scripts/pids/caddy.pid
end script

5. What did you expect to see?
Tested via generic phpinfo file in 4 locations, and expected all 4 to print the source code.
phpinfo.php
INDEX.php
index.php.phpinfo.php
index.phptest/phpinfo.php

6. What did you see instead (give full error messages and/or log)?
The last 2 (index.php.phpinfo.php and index.phptest/phpinfo.php) executed the php code, and showed the phpinfo page.
(Do I really need to paste 4 http 200 logs?)
But it seems like caddy only seems to say "Does it start with this?" and if it does, runs the code: While in 99% of environments this is fine, there should be some way to be more restrictive, to the point of specifying a single file or a single folder. For example, in the fastcgi page, it gives this:

Forward all requests in /blog to a PHP site (like WordPress) being served with php-fpm:
fastcgi /blog <IP_ADDRESS>:9000 php

But one wouldn't expect a file like /blogger/whatever.php to execute with that. (I didn't actually test setting it to a folder, but I can assume it'd do it from the behavior tested)
I'd assume that if I set to /blog and there's a folder named /blog, that anything in that folder is ok: but not a file named blogger.php. Which it seems it will.

Hi, thanks for the report.
This isn't a bug, as this is consistent with the path matching described in the docs (bottom of page).
If you want to match a directory specifically, suffix your base path with a /, in other words, /blog/.

Yeah, but there doesn't seem to be a way to match a specific FILE, not just a directory. (Also the fastcgi doc page should be updated to /blog/)

The Caddyfile is agnostic to what files exist on your system when it comes to path matching.
We should change the documentation to include the forward slash, you're right.
If you don't mind me asking, why do you have files called index.php.phpinfo.php and index.phptest/phpinfo.php? Part of the Caddy design is that it encourages using an elegant file/folder structure for your site.

It was kind of a "Hmm, I wonder if that actually works" moment of trying it out. And upon bringing it up on irc, a few people told me to submit a bug report just in case.

Ahh, gotcha :smile: Thanks for being thorough and taking the time to do that! I've updated the docs page locally so it will be deployed in the future.
Feel free to comment if there's anything else.


[Enhancement] Remove kvCQ batch write

Which Issue(s) This PR Fixes

Fixes #8698
Brief Description

How Did You Test This Change?

In order to minimize changes, we have opted to set the batch_size to 1 this time. However, this solution is not perfect, and we will need to use a more rigorous approach in the future to completely resolve this issue.
为了尽量少的变更，我们此次采用了batch_size改为1的方式，但这个方案不完美，后续还是要用更严谨的方式，彻底修复这个问题

To avoid changing the default logic of the currently deployed cluster, we will make the batch_size configurable and consider a complete optimization later. @RongtongJin
为了不改变现有已部署集群的默认逻辑，我们把batch_size改为可配置，后续在考虑彻底优化 @RongtongJin

Codecov Report
Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 47.40%. Comparing base (525f877) to head (d4655cb).

Files with missing lines
Patch %
Lines

...ache/rocketmq/store/config/MessageStoreConfig.java
66.66%
1 Missing :warning:

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8739      +/-   ##
=============================================
- Coverage      47.48%   47.40%   -0.09%
+ Complexity     11568    11553      -15
=============================================
  Files           1282     1282
  Lines          89813    89813
  Branches       11553    11553
=============================================
- Hits           42646    42572      -74
- Misses         41935    42000      +65
- Partials        5232     5241       +9

Flag
Coverage Δ

47.40% <90.00%> (-0.09%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

I made some modifications to the tool, and the actual effect of the tool is shown in the image.
修改了下工具，工具实际效果如图所示


Tree drawer improve

I found that the result of TreeDrawer do not match with the implement of python version.(You can checkout the difference between these in AVLTreeTest.cs)
After reading the code from bstsize_r.py, I found out that the int position_out = leftPosition + middle / 2; in your code missed an /2;
Meanwhile, I remove the branch(@"/\") display while a node without child nodes for a better result.

Awesome! The TreeDrawer now produces a more elegant output! 👍


clusters-list shows version info, os domain

Description

Updated the clusters-list command to display the deployed configuration version and the cluster's OpenSearch Domain name
Included a couple minor tweaks.  Updated README to reflect the existence of the config-update command.  Also, added a ENV variable in the Viewer container that has the Viewer's dashboard URL.

Tasks

https://github.com/arkime/aws-aio/issues/84

Testing

Updated unit tests
Ran the cluster-create and clusters-list commands to test.

(.venv) chelma@3c22fba4e266 aws-aio % ./manage_arkime.py clusters-list
2023-07-28 13:27:22 - Debug-level logs save to file: /Users/chelma/workspace/Arkime/aws-aio/manage_arkime/manage_arkime.log
2023-07-28 13:27:22 - Using AWS Credential Profile: default
2023-07-28 13:27:22 - Using AWS Region: default from AWS Config settings
2023-07-28 13:27:22 - Retrieving cluster details...
2023-07-28 13:27:26 - Deployed Clusters:
[
    {
        "cluster_name": "MyCluster",
        "opensearch_domain": "arkimedomain872-ri9cfzw6itps",
        "configuration_capture": {
            "aws_aio_version": "1",
            "config_version": "2",
            "md5_version": "13f556d965d6fd105425632066a74624",
            "source_version": "v0.1.1-10-gee5102d",
            "time_utc": "2023-07-26 17:45:35"
        },
        "configuration_viewer": {
            "aws_aio_version": "1",
            "config_version": "1",
            "md5_version": "e1fa07b3bde0479de0bdbe73995c8376",
            "source_version": "v0.1.1-9-g065772a",
            "time_utc": "2023-07-26 17:00:10"
        },
        "monitored_vpcs": []
    },
    {
        "cluster_name": "MyCluster3",
        "opensearch_domain": "arkimedomain872-1nzuztrqm7dl",
        "configuration_capture": {
            "aws_aio_version": "1",
            "config_version": "7",
            "md5_version": "64295265159ac577cf741bb4d1966fcc",
            "source_version": "v0.1.1-10-g3487548",
            "time_utc": "2023-07-27 18:26:23"
        },
        "configuration_viewer": {
            "aws_aio_version": "1",
            "config_version": "1",
            "md5_version": "be13e172a6440fcd6a7e73c8ae41457b",
            "source_version": "v0.1.1-7-g9c2d7ca",
            "time_utc": "2023-07-24 17:04:25"
        },
        "monitored_vpcs": [
            {
                "vpc_id": "vpc-008d258d7c536384b",
                "vni": "15"
            }
        ]
    }
]

License
I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

Merged in https://github.com/arkime/aws-aio/pull/100


fix: Add ansible 2.15 support

@SuperQ @paulfantom
FYI: added support for triggering role tests manually.
By default the roles are only tested if we detect any changes to the role files, but now if you add a label to the PR in the form of test-role-ROLENAME then that will trigger a test of the role ROLENAME


Remove ntfs junctions if they break

For some reason, rclone will crash and the symlinks ("ntfs junctions" as they call them) don't point anywhere, but can't be mounted over like a normal directory". This just forcefully removes those

@manics  Can you get SPSS to run under WINE? :D


Add better logging around DB migrations

When going back through logs, it would be helpful to see the DB version at a specific migration. This has come up when looking into issues with past migrations which were found to be faulty at a later point in time.
Basically, this just adds the current and new versions to the existing logging along with the clean/dirty flag.

Codecov Report
Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (d951d5f) 78.28% compared to head (bf6def0) 78.03%.

:exclamation: Current head bf6def0 differs from pull request most recent head 909552b. Consider uploading reports for the commit 909552b to get more accurate results

Files
Patch %
Lines

database/migrator.go
40.00%
7 Missing and 2 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
- Coverage   78.28%   78.03%   -0.26%
==========================================
  Files          42       42
  Lines        1695     1707      +12
==========================================
+ Hits         1327     1332       +5
- Misses        289      294       +5
- Partials       79       81       +2

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


docs: Add additional workflow for rebuilding old versions of documentation

Issue #, if available:
#1239
This change is required only for rebuilding old documentation and publish it to S3. Should be reverted when job is done.
Checklist

[ ] Meet tenets criteria
[ ] Update tests
[ ] Update docs
[ ] PR title follows conventional commit semantics

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Codecov Report
Patch and project coverage have no change.

Comparison is base (bca90c9) 78.57% compared to head (64a3e37) 78.57%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1371   +/-   ##
=========================================
  Coverage     78.57%   78.57%
  Complexity      651      651
=========================================
  Files            74       74
  Lines          2506     2506
  Branches        259      259
=========================================
  Hits           1969     1969
  Misses          455      455
  Partials         82       82

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Hey @kozub i'll close this for now, and we can re-open it when we have time to revisit


[improve][misc] Upgrade Netty to 4.1.87.Final

Motivation
Some relevant Netty bug fixes:

Improves compatibility with newer Linux distros

https://github.com/netty/netty/pull/13112

TLS 1.3 handshake issue fix:

issue: https://github.com/netty/netty/issues/13073
fixed by https://github.com/netty/netty/pull/13095

Modifications

Brings Netty Tcnative 2.0.56.Final
Also upgraded Netty's io_uring support to a compatible version 0.0.17.Final

Release notes:
https://netty.io/news/2023/01/12/4-1-87-Final.html
https://netty.io/news/2023/02/02/multiple_releases_incubator.html
Documentation

[ ] doc
[ ] doc-required
[x] doc-not-needed
[ ] doc-complete

/pulsarbot rerun-failure-checks

Codecov Report

Merging #19417 (5ca5403) into master (39dd1cd) will increase coverage by 30.18%.
The diff coverage is 29.03%.

@@              Coverage Diff              @@
##             master   #19417       +/-   ##
=============================================
+ Coverage     32.36%   62.55%   +30.18%
- Complexity     6355    25625    +19270
=============================================
  Files          1648     1829      +181
  Lines        123990   133915     +9925
  Branches      13523    14732     +1209
=============================================
+ Hits          40129    83770    +43641
+ Misses        77958    42365    -35593
- Partials       5903     7780     +1877

Flag
Coverage Δ

inttests
24.86% <19.03%> (+0.05%)
:arrow_up:

systests
25.44% <14.03%> (-0.25%)
:arrow_down:

unittests
59.75% <26.53%> (+42.21%)
:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

.../org/apache/pulsar/PulsarClusterMetadataSetup.java
16.96% <ø> (ø)

...pache/pulsar/broker/admin/impl/NamespacesBase.java
64.07% <0.00%> (+39.60%)
:arrow_up:

...r/delayed/bucket/BucketDelayedDeliveryTracker.java
0.00% <0.00%> (ø)

.../loadbalance/extensions/models/UnloadDecision.java
0.00% <0.00%> (ø)

...dbalance/extensions/scheduler/TransferShedder.java
0.00% <0.00%> (ø)

...e/pulsar/broker/service/TopicEventsDispatcher.java
32.25% <32.25%> (ø)

...va/org/apache/pulsar/broker/service/ServerCnx.java
52.39% <53.84%> (+15.29%)
:arrow_up:

...sar/broker/service/persistent/PersistentTopic.java
63.61% <69.23%> (+12.33%)
:arrow_up:

.../org/apache/pulsar/admin/cli/CmdTopicPolicies.java
89.56% <80.00%> (+89.56%)
:arrow_up:

...ava/org/apache/pulsar/admin/cli/CmdNamespaces.java
76.15% <83.33%> (+76.15%)
:arrow_up:

... and 1230 more