DOI-BOR / ostrich

An optimization toolkit for model calibration
GNU General Public License v2.0
1 stars 1 forks source link

Lsmatott #39

Closed dloney closed 2 years ago

dloney commented 2 years ago

Requesting approval from @lsmatott to make sure these are the most recent changes to the branch.

dloney commented 2 years ago

Quick questions so I understand the intent of the changes. Is there a reason to still support GCC5? I'm surprised that even works with the C++17 filesystem dependencies.

You've added components for the parameter initialization. Is that something that would make more sense to add to the dev branch, rather than adding functionality into the old codebase? Otherwise it will add to items that need to be migrated to dev in the future before the full change over.

lsmatott commented 2 years ago

HI Drew,

The dev branch has reduced functionality relative to the main branch - is that correct?

Thanks,

--- Shawn

From: Drew Allan Loney @.> Sent: Wednesday, March 2, 2022 10:46 PM To: usbr/ostrich @.> Cc: Loren Matott @.>; Mention @.> Subject: Re: [usbr/ostrich] Lsmatott (PR #39)

Quick questions so I understand the intent of the changes. Is there a reason to still support GCC5? I'm surprised that even works with the C++17 filesystem dependencies.

You've added components for the parameter initialization. Is that something that would make more sense to add to the dev branch, rather than adding functionality into the old codebase? Otherwise it will add to items that need to be migrated to dev in the future before the full change over.

- Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fusbr%2Fostrich%2Fpull%2F39%23issuecomment-1057636464&data=04%7C01%7Clsmatott%40buffalo.edu%7C99780fd2d0634feb392608d9fcc85bc0%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637818759730428697%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=5VcEeo6gk9Klv7uGcDTLVFBBPejKUp9lpTbgHJ6iTMM%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAB6LJVDRPF4A6FIGC2RI6SLU6AYYFANCNFSM5L4ETXHQ&data=04%7C01%7Clsmatott%40buffalo.edu%7C99780fd2d0634feb392608d9fcc85bc0%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637818759730428697%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=B4p205CUW%2FhcinnzXOskvxx1p1z6qH5gZ58h%2BXA%2B42Y%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Clsmatott%40buffalo.edu%7C99780fd2d0634feb392608d9fcc85bc0%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637818759730428697%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Yr8BTo4YIcNxq3U1hKn%2B1xPyK1ogfZjck87hk5FdBsg%3D&reserved=0 or Androidhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Clsmatott%40buffalo.edu%7C99780fd2d0634feb392608d9fcc85bc0%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637818759730428697%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=gKogsDZBm8kZLD%2Fk75U%2FPOkMRkH7E2fnzag644eXmrk%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.**@.>>

dloney commented 2 years ago

That's correct. I've been migrating algorithms to the new MPI engine as I get the bandwidth to do so.

lsmatott commented 2 years ago

Ok - my changes are in support of some research projects underway at UB. It will require working versions of basically all of the optimization algorithms, including parallel functionality, so working from a branch of the main repo seems like the most convenient choice. The good news is that they won't be using GML and that seems to have been the focus of the "dev" branch so far.

Is there a command I can run to identify merge conflicts between "lsmatott" and "dev" branches?

Also, what is the new MPI engine?

Thanks,

---- Shawn

From: Drew Allan Loney @.> Sent: Thursday, March 3, 2022 9:45 AM To: usbr/ostrich @.> Cc: Loren Matott @.>; Mention @.> Subject: Re: [usbr/ostrich] Lsmatott (PR #39)

That's correct. I've been migrating algorithms to the new MPI engine as I get the bandwidth to do so.

- Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fusbr%2Fostrich%2Fpull%2F39%23issuecomment-1058112943&data=04%7C01%7Clsmatott%40buffalo.edu%7C66df8cd11fe148a6ed7c08d9fd24572d%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819154786572785%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=XHVkn0d8FluJ%2FzPeNPy7pRS7eCriln6o2R4WopUSVEM%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAB6LJVBTYCO3VFHH5SPW5TDU6DF5JANCNFSM5L4ETXHQ&data=04%7C01%7Clsmatott%40buffalo.edu%7C66df8cd11fe148a6ed7c08d9fd24572d%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819154786572785%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=K5E2RPKaOo6PwGZAcUoZfEi5fDkL6j8pDHVDwlBidUc%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Clsmatott%40buffalo.edu%7C66df8cd11fe148a6ed7c08d9fd24572d%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819154786572785%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=e%2FCLcBcdENHCP44jB11iL8e%2BauMeV6uhFCrMT2FnnU8%3D&reserved=0 or Androidhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Clsmatott%40buffalo.edu%7C66df8cd11fe148a6ed7c08d9fd24572d%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819154786572785%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9rWU2z%2Bq%2FkkXAQ8BeHSKItfzOkTIBT6v8rrmoY21xCw%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.**@.>>

dloney commented 2 years ago

I thought we were going to transition to forking workflow to better manage merge conflicts.

The current version of Ostrich replicates the full version of memory and algorithm space between all workers and synchronizes the full results between all of them. That's not memory/compute efficient and isn't scalable. It also requires custom MPI logic within each algorithm.

The MPI engine on dev sets up a primary and secondary worker configuration. The primary worker has the algorithm logic and the secondary workers have just enough logic to do the model solves. That decouples the MPI logic from the algorithm logic, reducing the memory and compute usage. It also allows for reuse of the MPI solver logic. If the changes you're planning to make are going to be that extensive across all the algorithms, I'd recommend doing them on dev.

lsmatott commented 2 years ago

Hi Drew,

The changes will be applied to the "InitFromFile" portion of each algorithm.

Is there a way to promote my current branch into a fork? If not, I can save off my local copy of the branch and then overlay the latest round of changes onto a fork.

Thanks,

--- Shawn

From: Drew Allan Loney @.> Sent: Thursday, March 3, 2022 11:55 AM To: usbr/ostrich @.> Cc: Loren Matott @.>; Mention @.> Subject: Re: [usbr/ostrich] Lsmatott (PR #39)

I thought we were going to transition to forking workflow to better manage merge conflicts.

The current version of Ostrich replicates the full version of memory and algorithm space between all workers and synchronizes the full results between all of them. That's not memory/compute efficient and isn't scalable. It also requires custom MPI logic within each algorithm.

The MPI engine on dev sets up a primary and secondary worker configuration. The primary worker has the algorithm logic and the secondary workers have just enough logic to do the model solves. That decouples the MPI logic from the algorithm logic, reducing the memory and compute usage. It also allows for reuse of the MPI solver logic. If the changes you're planning to make are going to be that extensive across all the algorithms, I'd recommend doing them on dev.

- Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fusbr%2Fostrich%2Fpull%2F39%23issuecomment-1058257558&data=04%7C01%7Clsmatott%40buffalo.edu%7C1f7a61b53ba042d3ad7908d9fd368b66%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819233086961329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=OR%2BN65ZRt6esm8LhL9N%2F9M%2B%2Bi4LPkLCC4OuZJfIoIOk%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAB6LJVGETBKLIHI53E7QV4LU6DVF5ANCNFSM5L4ETXHQ&data=04%7C01%7Clsmatott%40buffalo.edu%7C1f7a61b53ba042d3ad7908d9fd368b66%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819233086961329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=A66wDBwQWFkobBcU7rk77uxwta3yJemdN4M8aOfTykk%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Clsmatott%40buffalo.edu%7C1f7a61b53ba042d3ad7908d9fd368b66%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819233086961329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=KnAO%2BfzI7f1nIa6xRRdRkQ0MZLDacPPwCgWfe7ekKBo%3D&reserved=0 or Androidhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Clsmatott%40buffalo.edu%7C1f7a61b53ba042d3ad7908d9fd368b66%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819233086961329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9OAn2%2F2Al1Ik79Hvez8t6r%2BCcKDimVzRhvSMUE0jCh4%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.**@.>>

dloney commented 2 years ago

If you make a fork of the project, you should get this branch to come with it. You should be able to check the branch back out and make pull requests back into this repository.

I'm not opposed to adding new features into the current code base, but we should mirror the functionality over onto dev so we can enable the future transition to dev.

lsmatott commented 2 years ago

Hi Drew,

Sounds like a plan --- I'll fork the project this weekend and start using that approach.

Should I manually mirror the functionality in the dev branch of my fork? Or is that something that git commands can help with?

Thanks,

--- Shawn

From: Drew Allan Loney @.> Sent: Thursday, March 3, 2022 12:46 PM To: usbr/ostrich @.> Cc: Loren Matott @.>; Mention @.> Subject: Re: [usbr/ostrich] Lsmatott (PR #39)

If you make a fork of the project, you should get this branch to come with it. You should be able to check the branch back out and make pull requests back into this repository.

I'm not opposed to adding new features into the current code base, but we should mirror the functionality over onto dev so we can enable the future transition to dev.

- Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fusbr%2Fostrich%2Fpull%2F39%23issuecomment-1058315757&data=04%7C01%7Clsmatott%40buffalo.edu%7C5e8563dbabcc469c146b08d9fd3dad13%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819263606170672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bS2CUeoXbSqxYXUF0E0bMNuqHC6VYsL%2Fhmz%2Fo%2F7hyMY%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAB6LJVFEDIB3P2A4O3SP4ZDU6D3FNANCNFSM5L4ETXHQ&data=04%7C01%7Clsmatott%40buffalo.edu%7C5e8563dbabcc469c146b08d9fd3dad13%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819263606170672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=lh%2BlCFi%2FgQ%2B4MXW9JtwRkKXOjt4bMo8llBT4BpWiQoA%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Clsmatott%40buffalo.edu%7C5e8563dbabcc469c146b08d9fd3dad13%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819263606170672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=MXG1Lb2ZfNMEP0%2B%2BFTPtnatmCtkrupsI%2BpxeBVHVt1Y%3D&reserved=0 or Androidhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Clsmatott%40buffalo.edu%7C5e8563dbabcc469c146b08d9fd3dad13%7C96464a8af8ed40b199e25f6b50a20250%7C0%7C0%7C637819263606170672%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=LvuHTqqiLw8jaKIhIDgH7tHA0ah%2BI1uYKFrQNFWNnJQ%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.**@.>>