-1

Here is the question: enter image description here

exmaple:

5
12 34 10 7 8
3
1 2
2 3
3 1

Output:

56
  • Groups are formed as: [1,2,3][4][5].
  • Maximum collection is 56 (collected by volunteer 1, 2, and 3).

My logic was to run a dfs and store the maximum value among all components in graph as my ans,

Here is my code:

#include <iostream>
#include <vector>
#include <unordered_map>

using namespace std;

int dfs(int v, int values[], unordered_map<int, vector<int>> adj, int visited[])
{
    int ans = 0;
    if (visited[v] == 0)
    {
        visited[v] = 1;
        for (auto j : adj[v])
        {
            if (visited[j] == 0)
            {
                ans = ans + dfs(j, values, adj, visited);
            }
        }
        return ans + values[v];
    }
    return 0;
}

void solve()
{
    int n;
    cin >> n;
    int v[n];
    int visited[n];
    for (int i = 0; i < n; i++)
    {
        visited[i] = 0;
    }
    //vector<vector<int>> adj(n);
    unordered_map<int, vector<int>> adj;

    for (int i = 0; i < n; i++)
    {
        cin >> v[i];
    }
    int p;
    int ans = 0;
    cin >> p;
    while (p--)
    {
        int f, s;
        cin >> f >> s;
        f = f - 1;
        s = s - 1;
        adj[f].push_back(s);
        adj[s].push_back(f);
    }
    for (int i = 0; i < n; i++)
    {
        if (visited[i] == 0)
        {
            int val = dfs(i, v, adj, visited);
            if (val > ans)
            {
                ans = val;
            }
        }
    }
    // for (int i = 0; i < n; i++)
    // {
    //     cout << i << " ";
    //     for (auto j : adj[i])
    //     {
    //         cout << j << " ";
    //     }
    //     cout << endl;
    // }
    cout << ans << endl;
}

int main()
{
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    cout.tie(NULL);

    // #ifndef ONLINE_JUDGE
    //     freopen("input.txt", "r", stdin);

    //     freopen("output.txt", "w", stdout);

    // #endif // ONLINE_JUDGE

    int t = 1;
    //cin >> t;
    while (t--)
    {
        solve();
    }
}

However my solution was giving a memory limited exceeded error.

Then I tried with union-find data structure, but it was giving wrong answer. Here is my code:

#include <iostream>
#include <vector>
#include <unordered_map>

using namespace std;

vector<int> parent;
vector<int> values;
vector<int> r;

int find(int s)
{
    if (parent[s] == s)
    {
        return s;
    }
    return parent[s] = find(parent[s]);
}

void unite(int a, int b)
{
    a = find(a);
    b = find(b);
    if (a != b)
    {
        if (r[a] < r[b])
        {
            parent[a] = parent[b];
            r[b] = r[b] + r[a];
        }
        else
        {
            parent[b] = parent[a];
            r[a] = r[a] + r[b];
        }
    }
}

void solve()
{
    int n;
    cin >> n;
    parent.resize(n);
    values.resize(n);
    r.resize(n);
    int ans = 0;

    for (int i = 0; i < n; i++)
    {
        parent[i] = i;
    }
    for (int i = 0; i < n; i++)
    {
        cin >> values[i];
    }
    for (int i = 0; i < n; i++)
    {
        r[i] = 1;
    }
    int p;
    cin >> p;
    unordered_map<int, int> m;
    while (p--)
    {
        int f, s;
        cin >> f >> s;
        f = f - 1;
        s = s - 1;
        int f1 = find(f);
        int f2 = find(s);

        if (f1 != f2)
        {
            unite(f1, f2);
        }
    }

    for (int i = 0; i < n; i++)
    {
        if (m[parent[i]])
        {
            m[parent[i]] = m[parent[i]] + values[i];
            ans = max(ans, m[parent[i]]);
        }
        else
        {
            m[parent[i]] = values[i];
            ans = max(ans, m[parent[i]]);
        }
    }

    cout << ans << endl;
}

int main()
{
        ios_base::sync_with_stdio(false);
        cin.tie(NULL);
        cout.tie(NULL);
    #ifndef ONLINE_JUDGE
        freopen("input.txt", "r", stdin);

        freopen("output.txt", "w", stdout);

    #endif // ONLINE_JUDGE
   

    solve();
}

Can someone guide me where I am wrong?

biqarboy
  • 852
  • 6
  • 15
Chaitanya Gujarathi
  • 137
  • 3
  • 9

2 Answers2

1

Unfortunately I cannot follow your approach. I am very sorry for that.

Anyway, I would propose to use a different algorithm, based on "Cyclic Permutation". The question at hand cries for using this method. For a description, you may read the article in Wikipedia here.

The additional stuff with "funds collection" is just a further indirection and is not relevant for the problem to solve.

The basic task is to create groups or cyles.

The approach described in the Wikipedia article is using the Two Line notation. And since in the given question, only indexes are used, life will even be more easy in the end. I forget about the 1-based start-index here for the moment and will simply use offsets in the later program.

So, how will the given input look in 2 line notation?

1 2 3 4 5
2 3 1 4 5

And, how can we find the groups and cycles using this notation. We start to iterate over all values in the first line. For the first index 1, we store the data, then look under the 1 and find a 2. This 2, we search again in the first line, store it, and look under it. There we find the 3. Store the 3 and look under it. We find a 1. Search for the 1 and try to store it, but, it is already there. So stop the evaluation for the first value of the first line. The resulting goup will be 1-2-3.

Then continue with the next value in the first line. With the above described approach, we will get 2-3-1. And for the next value in line 1 we will get 3-1-2, then 4 only and 5 only. So, the cycles, groups are (1,2,3), (4), (5).

Very straightforward. In order to avoid duplicate cycles / groups, we store the cyles in a std::set. So all cylce / group members will be unique and sorted. That is important, if we want later to optimize the algorithm. We can check, if a entry from the first line is already a member of a cycle / group. Then this evaluation is not necessary. This can save effort for big groups of volunteers.

On the other hand, we need to store all cycles / groups and search within this collection. So, this will cost also space and time. I am not sure, what is better at the moment. But I leave the check in the below shown example code.

Next optimization. We do not need 2 lines at all. Because only indices are stored. So, the first line is always simply the index 1...Number-of-volunteers. And if we read in the pair values, then we store the second value of the pair at the index, denoted by the first value of the pair. And to not miss the rest of the values, we initailze the basice std::::vector with std::iota in the beginning.

So, we initialze our std::vector. Content is 1,2,3,4,5, then we read 1,2. Meaning put the second value 2 at index one. Content is now 2,2,3,4,5. Then read 2,3. Meaning, put second value of the pair 3 at position 2. Content is now 2,3,3,4,5. Next is 3,1. So put 1 at position 3. Content is now 2,3,1,4,5. And because there is no more input, this is the final input sequence.

And for the evaluation, already described above. We iterate now over all elements from this std::vector. We start with index 1 and store that value in the cylcle / group. At index 1, we find a 2. The new index will be 2. Add that to the cycle / group (now: 1,2). What is at index 2? Right, the 3. Add that to the cycle / group (now: 1,2,3). At index 3, we find the value 1. That is already in our cycle / group, so we stop the evaluation for the first index. The first cycle / group will be (1,2,3).

Then, we go to the next index, and get 2,3,1. We stoe the values in a std::set and hence we will have again 1,2,3. No need for further evaluation.

And so on and so on.

After having found a cycle / group, we calculate the sum of collections, based on the indices in the cycle / group. And, compare this sum with the current max Value, which we will update, if necessary.

Basically, we do not need to store all cycles / groups, but it migth improve execution speed (though, I am not sure).

The example code could look like this:

#include <vector>
#include <sstream>
#include <iostream>
#include <algorithm>
#include <iterator>
#include <numeric>
#include <set>
#include <limits>

// Simulation of input data
std::istringstream inputData{R"(5
12 34 10 7 8
3
1 2
2 3
3 1)"};

// Functionality
int main() {

    // All the following is the input handling -----------------------------------------------------------

    size_t numberOfVolunteers{}; inputData >> numberOfVolunteers;   // Read group size

    std::vector<int>collections(numberOfVolunteers);                // Get all "collection" values
    std::copy_n(std::istream_iterator<int>(inputData), numberOfVolunteers, collections.begin());

    size_t numberOfPairs{}; inputData >> numberOfPairs;             // Number of pairs to read

    std::vector<size_t> permutations(numberOfVolunteers);           // Initialize our permutations vector
    std::iota(permutations.begin(), permutations.end(), 0);

    for (size_t i{}; i < numberOfPairs; ++i) {                      // Get all cyclic permutations
        size_t index{}; size_t target{}; inputData >> index >> target;
        permutations[index - 1] = target - 1;
    }

    // Input done, now start algorithm -----------------------------------------------------------
        
    std::vector<std::set<size_t>> allCycles{};          // All unique cycles / group of volunteers
    int maxSum{ std::numeric_limits<int>::min() };      // And thsi will be the result of the program

    // Go through all entries of the whole group
    for (size_t currentIndex = 0U; (currentIndex < numberOfVolunteers); ++currentIndex) {

        // If this menmber is already part of a cylce/group, then do not make an evaluation again
        if (std::count_if(allCycles.begin(), allCycles.end(), [&](const std::set<size_t>& c) {return c.count(currentIndex); }) == 0) {

            std::set<size_t> cycle{};                   // Here we will store one cycle / group of volunteers
            size_t index = currentIndex;                // And we will follow the chain of successors starting with current index

            // As long as we find successors
            for (bool insertResult{ true }; insertResult; ) {
                const auto& [newElement, insertOk] = cycle.insert(index);   // Insert new group member
                index = permutations[index];                                // Set next successor
                insertResult = insertOk;                                    // Continue?
            }
            //Calculate the sum of the collections for this Cycle / Group
            int sum{};  for (size_t index : cycle) sum += collections[index];
            maxSum = std::max(sum, maxSum);

            // Debug Output. Please uncomment, if you want to see debug output
            std::cout << "Cycle: "; for (size_t index : cycle) std::cout << index + 1 << ' '; std::cout << " Sum: " << sum << "  MaxSum : " << maxSum << '\n';

            // Save current cycle
            allCycles.emplace_back(std::move(cycle));
        }
        std::cout << maxSum << '\n';    // Show result
    }
    return 0;
}

If memory consumption is an issue, then I would omit all references and lines which contain "allCycles"

Developed compiled and tested with Microsoft Visual Studio Community 2019, Version 16.8.2

Additionally compiled and tested with gcc 10.2 and clang 11.0.1

Language C++17

A M
  • 14,694
  • 5
  • 19
  • 44
0

There are couple of issues in your first approach (finding groups by DFS):

  1. You declared (i.e., allocated memory) for v, visited and adj inside the function solve(). This is absolutely fine if your program will run for a single test-case. From your code, I am assuming your code will expect multiple test-cases. Now, every time you call the solve() method, you will allocate memory for v, visited and adj and when your program returned to the main() function, those memory will not released.
    • The easiest fix will be if you declare those variable in the global-scope and reuse them for every test-cases.
  2. In dfs() function, the function parameters are: 1 integer, 2 n-sized array, and 1 map containing the edge information. You pass these parameters using call-by-value, meaning every time dfs() function is called, these parameters will be copied locally to that function.
    • The fix is: (i) either use call-by-reference, or (ii) keep these parameters in global-scope, so that you do not need to pass them as a parameter every time you call dfs() function.

Here I have fixed the issues, have a look:

#include <iostream>
#include <vector>
#include <unordered_map>

using namespace std;

#define MAX_NODE 10005

unordered_map<int, vector<int>> adj;
int values[MAX_NODE];
int visited[MAX_NODE];

int dfs(int v)
{
    int ans = values[v];
    visited[v] = 1;
    for (auto j : adj[v])
    {
        if (visited[j] == 0)
        {
            ans = ans + dfs(j);
        }
    }
    return ans;
}

void solve()
{
    int n;
    cin >> n;
    for (int i = 0; i < n; i++)
    {
        cin >> values[i];
        visited[i] = 0;
    }

    int p;
    int ans = 0;
    cin >> p;
    while (p--)
    {
        int f, s;
        cin >> f >> s;
        f = f - 1;
        s = s - 1;
        adj[f].push_back(s);
        adj[s].push_back(f);
    }
    
    for (int i = 0; i < n; i++)
    {
        if (visited[i] == 0)
        {
            int val = dfs(i);
            if (val > ans)
            {
                ans = val;
            }
        }
    }
    
    cout << ans << endl;
    adj.clear();
}

int main()
{
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    cout.tie(NULL);

    // #ifndef ONLINE_JUDGE
    //     freopen("input.txt", "r", stdin);

    //     freopen("output.txt", "w", stdout);

    // #endif // ONLINE_JUDGE

    int t = 1;
    //cin >> t;
    while (t--)
    {
        solve();
    }
}

In your second approach (finding groups by union-find):

  1. There is an issue in your union-by-rank. According to geeksforgeeks:
    • Attach smaller rank tree under root of high rank tree
    • If ranks are same, then make one as root and increment its rank by one
  2. Before finding the group-id (by checking parent), make an extra round of compression. It will help fixing the issue if there is any uncompressed path in the union-find tree.
  3. Do not declare heavy data-structure (like you used map) inside a local method, it will kill you for multiple test-case program. I detailed described about the impact in your DFS-solution analysis.

Here I have fixed the issues, have a look:

#include <iostream>
#include <vector>
#include <unordered_map>

using namespace std;

vector<int> parent;
vector<int> values;
vector<int> r;
unordered_map<int, int> m;

int find(int s)
{
    if (parent[s] == s)
    {
        return s;
    }
    return parent[s] = find(parent[s]);
}

void unite(int a, int b)
{
    a = find(a);
    b = find(b);
    if (a != b)
    {
        if (r[a] < r[b])
        {
            parent[a] = b;
        }
        else if(r[a] > r[b])
        {
            parent[b] = a;
        }
        else
        {
            parent[b] = a;
            r[a] += 1;
        }
    }
}

void solve()
{
    int n;
    cin >> n;
    parent.resize(n);
    values.resize(n);
    r.resize(n);
    int ans = 0;

    for (int i = 0; i < n; i++)
    {
        cin >> values[i];
        parent[i] = i;
        r[i] = 1;
    }
    
    int p;
    cin >> p;
    while (p--)
    {
        int f, s;
        cin >> f >> s;
        f = f - 1;
        s = s - 1;
        int f1 = find(f);
        int f2 = find(s);

        if (f1 != f2)
        {
            unite(f1, f2);
        }
    }
    
    // compression one more time to confirm the integrary of group number
    for (int i = 0; i < n; i++) find(i);

    for (int i = 0; i < n; i++)
    {
        if (m.find(parent[i]) != m.end())
        {
            m[parent[i]] = m[parent[i]] + values[i];
            ans = max(ans, m[parent[i]]);
        }
        else
        {
            m[parent[i]] = values[i];
            ans = max(ans, m[parent[i]]);
        }
    }

    cout << ans << endl;
    m.clear();
}

int main()
{
        ios_base::sync_with_stdio(false);
        cin.tie(NULL);
        cout.tie(NULL);
    // #ifndef ONLINE_JUDGE
    //     freopen("input.txt", "r", stdin);

    //     freopen("output.txt", "w", stdout);

    // #endif // ONLINE_JUDGE
   

    solve();
}
biqarboy
  • 852
  • 6
  • 15